Getting max and min values from Array

Question

My code works; I am just seeing if there's an optimized way to write these methods or even better perhaps combine them (since majority of their contents look similar).

public class IntegerUtils {

    public static int getMaxValue(int[] array) {
        int value = Integer.MIN_VALUE;

        if (array.length <= 0) {
            throw new IllegalArgumentException("Array is empty.");
        }

        for (int i = 0; i < array.length; i++) {
            if (array[i] > value) {
                value = array[i];
            }
        }
        return value;
    }

    public static int getMinValue(int[] array) {
        int value = Integer.MAX_VALUE;

        if (array.length <= 0) {
            throw new IllegalArgumentException("Array is empty.");
        }

        for (int i=0; i < array.length; i++) {
            if (array[i] < value) {
                value = array[i];
            }
        }
        return value;
    }   
}

Here's my unit test (which works as well):

import org.junit.Test;

public class IntegerUtilsTest {

    int[] array = new int[] {1, 2, 3, 58, 47, 229, 40};

    @Test
    public void shouldBeMaxValue() {
        int maxValue = IntegerUtils.getMaxValue(array);
        assert(maxValue == 229);
    }

    @Test
    public void shouldBeMinValue() {
        int minValue = IntegerUtils.getMinValue(array);
        assert(minValue == 1);
    }

    @Test(expected = IllegalArgumentException.class)
    public void shouldBeIllegalArgumentException() {
        int[] emptyArray = new int[] {};
        int maxValue = IntegerUtils.getMaxValue(emptyArray);
        int minValue = IntegerUtils.getMinValue(emptyArray);
    }
}

Just seeking feedback regarding optimization and / or code elegance.

Answer 1

Real skill means knowing your libraries. Instead of 20 lines of handcrafted code, I'd rather go for:

int max = Arrays.stream(array).max().orElseThrow(() -> new IllegalArgumentException("Array is empty"));

... the same for min.

As a general advice: Java is a very mature language, and chances are good that every "simple everyday task" is already solved in the library. (Also note that most books on Java are from the pre-java-8 era and may be considered somewhat outdated today.)

Answer 2

I suggest the use of JUnit's assert methods, i.e. assertTrue, assertEquals, etc.

@Test
public void shouldBeMaxValue() {
    int maxValue = IntegerUtils.getMaxValue(array);
    Assert.assertEquals(229, maxValue);

    // or Assert.assertTrue(maxValue == 229);
}

I prefer assertEquals over assertTrue due to the better message in case of failure:

Assert.assertEquals(229, maxValue); 
// java.lang.AssertionError: expected:<229> but was:<100>

Assert.assertTrue(229 == maxValue); 
// java.lang.AssertionError

Assert.assertThat() is another (more readable) solution (and my favorite):

Assert.assertThat(maxValue, is(229));

Answer 3

The main code looks great, except for the inconsistent formatting. Let your IDE format your code for you.

To make the functions easier to use, you could replace int[] array with int... array, so that they can be called like getMinValue(1, -5).

You should add some tests for negative numbers, and for MIN_VALUE and MAX_VALUE. There should always be tests for some "normal" values, as well as tests for extreme values or special values.

Your test for the empty array must be split into two separate tests since the last line is currently not executed. When you run your tests "with coverage" (something your IDE should provide), you can see those lines of code that are not tested.

Instead of assert, you should call the method Assert.assertEquals. This way, when one of the assertions fails, you get a nice error message that includes the expected and the actual value.

If you didn't know how getMinValue is implemented, you would need additional tests in which the minimum value is at the beginning of the array, or in the middle of the array, or at the end of the array. This is because a typical implementation will initialize value with array[0] instead of MAX_VALUE and your tests have to make sure that these special cases are handled correctly.

Answer 4

I actually combined 2 methods to become one to reduce the repetition. I would recommend to use java's stream for cleaner and elegance code.

Not very elegance solution, especially the boolean part, but i tried, just provide an alternative solution or idea to the OP on how to combine both of the methods, maybe the usage of getMinOrMax(array, "min") is better than a boolean.

 public static int getMinOrMax(int[] array, boolean isMin) {
     if (array.length <= 0) {
         throw new IllegalArgumentException("Array is empty.");
     }

     int comparedValue = array[0];
     for (int i = 1; i < array.length; i++) {

         if (isMin && array[i] < comparedValue) {
             comparedValue = array[i];
         }
         if (!isMin && array[i] > comparedValue) {
             comparedValue = array[i];
         }
     }
     return comparedValue;
 }