Regarding your current code:
Public or private
You have three public static
variables, but your findMissingNumbers
method is private
. It would be better having it the other way around. You wouldn't want outside code to modify your static variables, those are meant to be used only by your findMissingNumbers
method.
The findMissingNumbers
method that you provide is meant to be a method that provides some calculations, it should be able to be called from other classes - hence it should be public
.
Naming
boolean flag = false;
It's a boolean, of course it's a flag! What is the flag used for? Reading this variable name now doesn't help me understand the variable's purpose at all. It should describe why it is being used.
Static variables
public static int count = 0;
public static int position = 0;
public static boolean flag = false;
Having static variables are usually only used for constants. Are these variables supposed to be constants? Constants should be marked final
. If you would mark them final you will quickly see that you will get compiler errors on count
and on flag
, because you modify those values inside the method. There is a good reason to not use them as public static variables!!
Consider what would happen if your main method would look like this:
public static void main(String[] args) {
int a[] = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 18, 20, 21, 23 };
findMissingNumbers(a, position);
int b[] = { 0, 1, 2, 3, 5, 6, 7, 8, 9, 10, 12, 13, 14, 16, 17, 18, 20, 21, 23 };
findMissingNumbers(b, position);
}
After the first call to findMissingNumbers
, your public static
variables would have changed, which would modify the behavior of the second method call. Actually, in this case it would change the behavior in such a big way that it would cause a StackOverflowError
. These two method calls should be independent.
By the way: I think that the position
parameter that you send to the method is not needed, the method should start searching at zero by default.
Solution:
Use more local variables. boolean flag
does not need to be a static variable. It should only be used within the method itself. It should be a local variable and initialized to false. Making this change actually solves the StackOverflowError
that would occur when calling this method twice.
The variables that is used inside the method, and gets changed, can be passed as parameters to the method. position
is already a parameter so the public static variable is in fact not used, other than to provide a starting value to the method.
To provide default values for the method, we can use a method that only calls the "real" method with some parameters.
Here is a better implementation with these flaws fixed of your code. Note that one of the findMissingNumbers
methods are public
(as it is the one that is meant to be called), while one remains private
. Also note the improved variable names, and how the parameters are passed between the methods - removing the big flaw of static variables.
public static void main(String[] args) {
int a[] = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 18, 20, 21, 23 };
findMissingNumbers(a);
int b[] = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 18, 20, 21, 23 };
findMissingNumbers(b);
}
private static void findMissingNumbers(int values[], int position, int count) {
boolean foundMissingNumber = false;
if (position == values.length - 1)
return;
for (; position < values[values.length - 1]; position++) {
if ((values[position] - count) != position) {
System.out.println("Missing Number: " + (position + count));
foundMissingNumber = true;
count++;
break;
}
}
if (foundMissingNumber) {
findMissingNumbers(values, position, count);
}
}
public static void findMissingNumbers(int values[]) {
findMissingNumbers(values, 0, 0);
}
Another improvement that would be helpful to make is to let your methods return the missing numbers, instead of doing the output inside the method. Consider for example if you would want to calculate the sum of the missing numbers, or the sum of the squares of the missing numbers. Currently, you would perhaps do some code duplication (which is a known code smell) to do this, but if you let your methods return the missing numbers, you would not need to do any code duplication. I'd recommend looking into a ArrayList<Integer>
for returning the values.