Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

An array is defined to be maxmin equal if it contains at least two different elements and the number of times the maximum value occur is the same as the number of times the minimum value occur. So {11, 4, 9, 11, 8, 5 , 4, 10} is maxmin equal, because the max value 11 and min value 4 both appear two times in the array.

  • if the input array is {} , isMaxMinEqual should return false (array must have at least two different elements) status = false;
  • if the input array is {2}, isMaxMinEqual should return false (array must have at least two different elements).
  • if the input array is {1,1,1,1,1,1}, isMaxMinEqual should return false (array must have at least two different elements).
  • if the input array is {2,4,6,8,11}, isMaxMinEqual should return true (Both max value(11) and min value 2 appear exactly one time).
  • if the input array is {-2,-4,-6,-8,-11}, isMaxMinEqual should return true(Both max value(-2) and min value -11 appear exaclty one time).

I wrote the class MaxMin which satisfied the above condition.

public class MaxMin {

public static void main(String args[]) {
    System.out.println("The result is: " + isMaxMinEqual(new int[]{2, 2}));
}

public static boolean isMaxMinEqual(int[] a) {
    boolean status = false;
    int count = 0;
    if (a.length < 2) {
        status = false;
        return status;
    }
    for (int i = 1; i < a.length; i++) {
        if (a[i - 1] != a[i]) {
            count++;
        }
    }

    int max = a[0];
    int min = a[0];
    for (int i = 0; i < a.length; i++) {
        if (a[i] > max) {
            max = a[i];
        } else if (a[i] < min) {
            min = a[i];
        }
    }

    int maxCount = 0;
    for (int j = 0; j < a.length; j++) {
        if (a[j] == max) {
            maxCount++;
        }
    }

    int minCount = 0;
    for (int j = 0; j < a.length; j++) {
        if (a[j] == min) {
            minCount++;
        }
    }
    System.out.println(count);
    System.out.println("max is: " + max);
    System.out.println("min is: " + min);
    System.out.println("Max Count is: " + maxCount);
    System.out.println("Min Count is: " + minCount);

    if (maxCount == minCount && count >= 1) {
        status = true;
    }

    return status;
}
}
share|improve this question
    
Are you on Java 8? – h.j.k. Jan 20 at 14:42
    
java version "1.7.0_91" OpenJDK Runtime Environment (IcedTea 2.6.3) (7u91-2.6.3-0ubuntu0.15.04.1) OpenJDK 64-Bit Server VM (build 24.91-b01, mixed mode) – user3789184 Jan 20 at 14:47
up vote 3 down vote accepted

There's no need to count the amount of times each item differs from the previous item, like you do here:

for (int i = 1; i < a.length; i++) {
    if (a[i - 1] != a[i]) {
        count++;
    }
}

You can just set a boolean and break; once you spot a single difference. The break will end the for loop directly. Then, if after that for loop, the boolean is not set and still false, you can just return false.

Like so:

boolean foundDifference = false;
for (int i = 1; i < a.length; i++) {
    if (a[i - 1] != a[i]) {
        foundDifference = true;
        break;
    }
}
if (!foundDifference){
    return false;
}

Next, you can combine your for loops to make the code faster.
First, here's the three for loops we want to combine.

int max = a[0];
int min = a[0];
for (int i = 0; i < a.length; i++) {
    if (a[i] > max) {
        max = a[i];
    } else if (a[i] < min) {
        min = a[i];
    }
}

int maxCount = 0;
for (int j = 0; j < a.length; j++) {
    if (a[j] == max) {
        maxCount++;
    }
}

int minCount = 0;
for (int j = 0; j < a.length; j++) {
    if (a[j] == min) {
        minCount++;
    }
}

Obviously, we can combine the bottom two the easiest since they only change the counts:

int max = a[0];
int min = a[0];
for (int i = 0; i < a.length; i++) {
    if (a[i] > max) {
        max = a[i];
    } else if (a[i] < min) {
        min = a[i];
    }
}

int maxCount = 0;
int minCount = 0;
for (int j = 0; j < a.length; j++) {
    if (a[j] == max) {
        maxCount++;
    }
    if (a[j] == min) {
        minCount++;
    }
}

And then we only have two for loops. But I think we can combine those two as well.

First, naively combine them:

int max = a[0];
int min = a[0];
int maxCount = 0;
int minCount = 0;
for (int i = 0; i < a.length; i++) {
    if (a[i] > max) {
        max = a[i];
    } else if (a[i] < min) {
        min = a[i];
    }
    if (a[i] == max) {
        maxCount++;
    }
    if (a[i] == min) {
        minCount++;
    }
}

Looks great, but if the min or the max changes, then we need to start over with counting.

So lets do that:

int max = a[0];
int min = a[0];
int maxCount = 0;
int minCount = 0;
for (int i = 0; i < a.length; i++) {
    if (a[i] > max) {
        max = a[i];
        maxCount = 0;
    } else if (a[i] < min) {
        min = a[i];
        minCount = 0;
    } 
    if (a[i] == max){
        maxCount++;
    }
    if (a[i] == min){
        minCount++;
    }
}

But, if we change the max or min, then we also already found 1 instance, so we should just start counting at 1. We also put the counting in an else statement so that we don't count them double:

int max = a[0];
int min = a[0];
int maxCount = 0;
int minCount = 0;
for (int i = 0; i < a.length; i++) {
    if (a[i] > max) {
        max = a[i];
        maxCount = 1;
    } else if (a[i] < min) {
        min = a[i];
        minCount = 1;
    } else {
        if (a[i] == max){
            maxCount++;
        }
        if (a[i] == min){
            minCount++;
        }
    }
}

This way you only iterate through the array once, instead of 3 times. So it's faster.


Then, at the end, you can directly return the result of the check, so instead of

if (maxCount == minCount && count >= 1) {
    status = true;
}

return status;

Just do

return maxCount == minCount && max != min;

We could remove the first check with the for loop, since I explicitly check at the end if max != min.

Making the end product something like this:

public static boolean isMaxMinEqual(int[] a) {
    if (a.length < 2) {
        return false;
    }

    int max = a[0];
    int min = a[0];
    int maxCount = 0;
    int minCount = 0;
    for (int i = 0; i < a.length; i++) {
        if (a[i] > max) {
            max = a[i];
            maxCount = 1;
        } else if (a[i] < min) {
            min = a[i];
            minCount = 1;
        } else {
            if (a[i] == max){
                maxCount++;
            }
            if (a[i] == min){
                minCount++;
            }
        }
    }
    return maxCount == minCount && max != min;
}
share|improve this answer
    
On your first suggestion, the foundDifference variable can be ommited, and just plainly return true on the condition, and false at the bottom of the function. – Kroltan Jan 20 at 18:18
    
@Kroltan [1, 1, 2] would return true if I were to return true because index 1 is different than index 2. Now, if I were to put this in a separate function, then yes. – Pimgd Jan 20 at 19:50
    
you're correct, I assumed it was a isolated function. – Kroltan Jan 20 at 19:51

Varags

Since Java 1.5, you can use varargs in your method so that you can pass in a sequence of elements, instead of an explicit array:

public static boolean isMaxMinEqual(int... values) {
    // use values like a normal int[] array
}

Variables assignment

You have declared status right at the start, but it's actually redundant as you can simply return from the method directly in the places where you use them:

if (a.length < 2) {
    // status = false;
    // return status;
    return false; // shorter and clearer
}
// ...
/*
if (maxCount == minCount && count >= 1) {
    status = true;
}
return status;
*/
return maxCount == minCount && count >= 1; // no need to set status = true

'Debug' statements

System.out.println(count);
System.out.println("max is: " + max);
System.out.println("min is: " + min);
System.out.println("Max Count is: " + maxCount);
System.out.println("Min Count is: " + minCount);

These look like 'debug' statements where the developer (or user) is manually checking that the counting is done correctly. Usually, this should be done through a logging framework (so that the verbosity can be adjusted), or be eliminated entirely. It does not serve a direct purpose as to the method's usage.

Iterations

Your approach has to iterate through the array three times:

  1. Find the min/max.
  2. Count the max.
  3. Count the min.

In fact, these can be done in one pass:

int min = Integer.MAX_VALUE;
int max = Integer.MIN_VALUE;
int minCounter = 0;
int maxCounter = 0;
for (int i : values) {
    if (i < min) {
        min = i;
        minCounter = 1;
    } else if (i == min) {
        minCounter++;
    }
    if (i > max) {
        max = i;
        maxCounter = 1;
    } else if (i == max) {
        maxCounter++;
    }
}
return minCounter == maxCounter && min != max;

The idea here is that when you need to reset your min/max values, you can just reset your counters to 1 and proceed, instead of having to iterate again.

If you do not mind extra ternary operators, you can sort of combine both if branches per min/max too:

int min = Integer.MAX_VALUE;
int max = Integer.MIN_VALUE;
int minCounter = 0;
int maxCounter = 0;
for (int i : values) {
    if (i <= min) {
        minCounter = 1 + (i == min ? minCounter : 0);
        min = i;
    }
    if (i >= max) {
        maxCounter = 1 + (i == max ? maxCounter : 0);
        max = i;
    }
}
return minCounter == maxCounter && min != max;
share|improve this answer
    
what is the purpose of reset of min/max values, using maxcounter = 1 and mincounter=1 when we are finding the max and min??? – user3789184 Jan 20 at 15:34
    
If we encounter a new min/max value, we should stop counting for the previous min/max and start from the current one. Hence minCounter/maxCounter = 1. :) – h.j.k. Jan 20 at 15:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.