You have already accepted an answer, and I don't code in java, but as a mathematician, I would like to point out that there is a very nice recursive algorithm which is much shorter; the Euclidean Algorithm: math.rutgers.edu/~greenfie/gs2004/euclid.html
–
jpreenDec 8 '13 at 14:54

(the common divisors are necessarily the divisors of the greatest common divisor)
–
jpreenDec 9 '13 at 18:37

3 Answers
3

First choose better variable names. I would name the Scanner type variable as scanner rather than x. Really, x, y, z are the worst variable name, which doesn't give any idea as to what they denote.

Secondly, I won't force user to pass larger number first, and then smaller number. I would take the burden of finding that out myself.

There is no need of storing all the divisors of both the numbers in two different arrays. Suppose you have to find common divisor of 2 and 2132340. Will you store all the divisors of 2132340, or simply check that the common divisor cannot be greater than the divisor of 2, and use that fact?

Another fact is, all the divisors of num1 will be less than or equal to num1 / 2. Now, if num1 < num2, then the common divisors all must be < num1 / 2. So, iterate the loop till min / 2.

Of course, you can move the code that finds the common divisor in a different method, and store them in a List<Integer>.

Lastly, it might be possible that the smaller number itself is a divisor of larger number. So, you need to store that too.

This might have been mentioned before but here it is anyway: You should format your code. Badly formatted code is hard to read - it might be the most efficient code in the world but it's still hard to read and maintain and easy to miss bugs or easy to add bugs.

It also shows a lack of attention to detail and programming is all about attention to detail. It basically demonstrates that not much care has been put into. Whenever you write some code imagine you'd have to show it as an example of your work for your next job interview.

You should name your classes and variables better. While i is acceptable for a loop variable l, x, y, and z should be renamed to reflect what their meaning is. It is especially confusing since x suggests to have a similar meaning to y and z while it actually is completely different (it represents a form of input rather than a number).

You should use an ArrayList rather than a fixed size array to store the divisors. If someone inputs 2147483647 then your program would try to allocate an array with 2,147,483,647 elements - approx. 8GB of memory even though it's a prime number.

If you change this then your check for the common divisors need to check the common subset s of both lists instead.

Your two for loops do the exact same thing except on different inputs. They should be refactored into one common method which performs the calculation and gets the number to check as input.

Your algorithm is buggy. For example if you check 2 and 4 you will not find 2 as a common divisor.