12

I've written two methods to find the smallest and largest int in an array, but they're nearly identical, so I feel like there should be some way to simplify this, perhaps as one method?

private int findMin(){ int min = arr[0]; for(int num : arr){ if(num<min) { min = num; } } return min; } private int findMax(){ int max = arr[0]; for(int num : arr){ if(num>max){ max = num; } } return max; }

I'm not sure how to approach this sort of issue, so I'd love to see your responses!

While this question on how to pass arithmetic operators to a method and this question on how to get both min and max value of Java 8 stream answer the literal programming problem, my question is on a more fundamental level about to how to approach the problem of methods doing similar things, and ways to compare arrays in general. The answers to this post have been significantly more helpful to me than the answers to those questions.

Improve this question
5
  • Related: stackoverflow.com/questions/41816264/… Commented Nov 3, 2022 at 4:11
  • Thanks for the suggestions Yay295 and Joe, they're interesting, but the answers to those aren't really what I was looking for. I've edited my post with an explanation. Commented Nov 3, 2022 at 15:01
  • Considering that each method is only a few lines of code, may I suggest that creating a simple combined method that invariably takes a parameter isn't particularly worth the (albeit slight) readability hit? Should you insist, @russianroadman's answer does a good job of creating a combined method and having the specific min and max methods call it. Best of both worlds! Commented Nov 3, 2022 at 19:26
  • If you need to trim lines or it will run faster, sure, but otherwise there's really no reason to combine similar methods. They act similar, but they return utterly different things. Commented Nov 4, 2022 at 5:19
  • What if the array's empty? Commented Nov 4, 2022 at 8:24

7 Answers 7

Reset to default
23

You can implement just one, say findMax, and pass it a Comparator that indicates how comparisons should be done:

private int findMax(Comparator<Integer> comparator) { int max = arr[0]; for (int num : arr) { if (comparator.compare(num, max) > 0) { max = num; } } return max; }

Pass Comparator.naturalOrder() for the natural ordering of integers, so you get the maximum.

Pass Comparator.reverseOrder() for the reverse ordering, so you get the minimum.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Interesting! I don't quite understand how comparators work yet, but it's going on my list of things to work on haha. I appreciate getting an answer that uses stuff I didnt even know existed.
As the name suggests, a Comparator is a "thing that compares". You can ask it to compare two things by calling its compare method, which returns -1 if the first is less than the second, 0 if they are equal, 1 if the first is greater than the second. You could implement your own Comparator, but the interface comes with several convenience methods that create commonly used instances like the ones I mentioned above.
It might be more readable to also provide a findMin method that just does return findMax(comparator.reversed());, and possibly overload both to default to the naturalOrder() comparator.
16

I would not recommend combine two responsibilities into one method

Instead, I would make two methods (max and min) that share same lines of code through a third method.

public int min(int[] array){ return most(array, (a, b) -> a < b) } public int max(int[] array){ return most(array, (a, b) -> a > b) } private int most(int[] array, Fuction2 compare){ int most = array[0]; for (int num : array) { if (compare(num, most)) { most = num; } } return most; }

This way you can easily change logic for one method, without changing another Also, it is easier to use and more readable, than using one method with parameter like "max=true"

Improve this answer

3 Comments

Best of both worlds here!
That would be my approach as well. Make the user interface as simple, clear, fool-proof and unambiguous as possible; hide complexity and genericity in the implementation. (You can still have an advanced API that allows power users access to generic functions though so that they don't get dumbed down and frustrated!)
Your Function2 should be java.util.BiPredicate<Integer, Integer>
16

Your methods are already available in IntStream that also deals with empty arrays (indeterminate extremes).

private OptionalInt findMin() { return IntStream.of(arr).min(); } private OptionalInt findMax() { return IntStream.of(arr).max(); }

And like your choice of throwing exceptions:

private int findMin() { return IntStream.of(arr).min().get(); } private int findMax() { return IntStream.of(arr).max().get(); }

Or as method reference:

Function<IntStream, OptionalInt> extreme = IntStream::max;

Also consider the parallel feature:

IntStream.of(arr).parallel().min();

@Yay295 reminded us of IntSummaryStatistics:

IntSummaryStatistics stats = new IntSummaryStatistics(); IntStream.of(arr).stream() .forEach(stats); // stats is an int consumer too.

or when doing more

IntSummaryStatistics stats = IntStream.of(arr) .collect(Collectors.summarizingInt(IntSummaryStatistics::new, IntSummaryStatistics::accept, IntSummaryStatistics::combine);

and then several numbers can be asked:

int min = stats.getMin(); int max = stats.getMax(); int avg = stats.getAverage(); int sum = stats.getSum();

Feedback of @draspa44

You can end any IntStream, LongStream or DoubleStream simply with .summaryStatistics(). I think this is easier to read than mutating stats or using accept and combine

IntSummaryStatistics stats = IntStream.of(arr) .summaryStatistics();
Improve this answer

7 Comments

There's also Int, Long, and Double SummaryStatistics. These stream collectors provide min, max, average, count, and sum.
@Yay295 IntSummaryStatistics does collect more, but is in fact even more appropriate as an answer because it combines min and max. Something for an answer!
Nice catch. It outruns my answer - I was not even aware of that class.
Warning: don't use parallel() unless you can prove with data that your code is too slow without it. It uses a global thread pool that cannot be easily changed (which could cause other code using this pool to take longer to run or to not run at all), and usually the normal stream is faster because the threads that parallel() uses have an overhead that, for small to medium-sized streams, just makes things slower.
You can end any IntStream, LongStream or DoubleStream simply with .summaryStatistics(). I think this is easier to read than mutating stats or using accept and combine.
|
8

If the number of int's is big and you most of the time need both min and max values, it probably makes sense to go through the list only once and evaluate both min and max at the same time.

Then there is just the question how to communicate two numbers when a function can only have one return value. So we create a special result class for this.

class MinMaxResult { public int min = 0; public int max = 0; } MinMaxResult findMinMax() { MinMaxResult result = new MinMaxResult(); result.min = arr[0]; result.max = arr[0]; for (int num: arr) { if (num < result.min) { result.min = num; } else if (num > result.max) { result.max = num; } } return result; }
Improve this answer

2 Comments

Even better, assuming this array only gets populated once, would be to maintain a min and max when adding the numbers to the array in the first place. Then there's no loop needed to get the values since you've already got them. A little harder if you're frequently modifying/removing elements. If that were the case, I'd use some kind of min/max heap structure instead of a flat array if performance is an issue.
I like that idea, although it would only work while values are added. Removing the one representing the max/min would invalidate the result. But only then a re-scan might be necessary.
3

You can also call the class methods Math.min and Math.max as follows and return the results in a record. Records are immutable classes and were introduced in Java 14.

record MinMax(int min, int max) { }; public static void main(String[] args) { int[] v = { 1, 1, 20, 3, 3, 10 }; MinMax result = findMinMax(v); System.out.println(result); // or System.out.println(result.min + ", "+ result.max); }

prints

MinMax[min=1, max=20] 1, 20

Just iterate over the array and apply the methods to each value and the current value of min or max.

public static MinMax findMinMax(int[] values) { int max = values[0]; int min = max; for (int i = 1; i < values.length; i++) { max = Math.max(max, values[i]); min = Math.min(min, values[i]); } return new MinMax(min, max); }
Improve this answer

Comments

1

Since there is really only one difference, you can pass a boolean to determine which value (min or max) you want. Something like this:

private int findExtreme(boolean findMin) { int solution = arr[0]; for(int num : arr){ if (findMin){ if(num<min){ solution = num; } } else { if(num>max){ solution = num; } } } return solution; }
Improve this answer

3 Comments

Thanks! That makes sense. I imagine I'll be using comments to make sure i don't mix up wether findMinOrMax(true) gets me min or max haha.
Possible, but bad for readability. From the call findMinOrMax(false) it's impossible to tell what the boolean argument means. An enum would be better.
Thanks for elaborating @Thomas, I hadn't heard of enums before reading through all the responses here, they seem like a very useful tool for making code significantly more readable.
1

You can pass in a boolean that is true to find the minimum or false to find the maximum. The trick is to compare the result of (num < best), which is either true or false, to the boolean argument.

  • If isMinimum is true then when (num < best) is true we have a new minimum.
  • If isMinimum is false then when (num < best) is false we have a new maximum (or a tie for maximum).
private int findExtreme(bool isMinimum) { int best = arr[0]; for (int num: arr) { if ((num < best) == isMinimum) { best = num; } } return best; }

Even better would be to use an enum. Boolean arguments can be hard to decipher just seeing a bare true or false in a function call. That would look like:

public enum Extreme { MINIMUM, MAXIMUM } private int find(Extreme extreme) { int best = arr[0]; for (int num: arr) { if ((num < best) == (extreme == Extreme.MINIMUM)) { best = num; } } return best; }
Improve this answer

2 Comments

Possible, but bad for readability. From the call findExtreme(false) it's impossible to tell what the boolean argument means. An enum would be better.
Right you are. I've added an enum.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.