To me it looks pretty readable
To me it doesn't.
Bugs:
The following method calls all incorrectly print 0
:
System.out.println(min(3, 2, 2));
System.out.println(min(3, 3, 3));
System.out.println(min(1, 3, 3));
System.out.println(min(4, 2, 4));
This is because, when taking a look at your original code, it is overly complicated.
if( a < b && a < c && b < c) result = a ;
else if( a < b && a < c && b > c) result = a ;
Does it really matter if b < c
or b > c
? No, it doesn't here. And if b == c
then neither of these current ones would be true which does the return 0
. So that's a giant bug waiting to happen.
So those two first if's should be shortened into:
if (a <= b && a <= c) return a;
Note that I'm using <=
here so that the case of all three having the same value gets handled correctly. Now also there's an early return so we don't need all these else
.
If we group the rest of your statements according to what they return, we have for return b
:
else if( a > b && a < c && b < c) result = b ;
else if( a > b && b < c && a > c) result = b ;
Which, if we always use b
as the first operand and completely ignore whether or not a < c
or a > c
(and again, a == c
is not handled here) we get:
if (b <= a && b <= c) return b;
Doing the same for c
:
if (c <= a && c <= b) return c;
As one of a
, b
or c
really has to be smallest, you can throw an exception if neither one of them is:
public static int min(int a, int b, int c) {
if (a <= b && a <= c) return a;
if (b <= a && b <= c) return b;
if (c <= a && c <= b) return c;
throw new AssertionError("No value is smallest, how did that happen?");
}
Or, if you don't like that exception:
public static int min(int a, int b, int c) {
if (a <= b && a <= c) return a;
if (b <= a && b <= c) return b;
return c;
}
This is in my opinion significantly better than your original version.
However, I would recommend Pimgd's solution, either an array or using chained Math.min
.
a < b && a < c && b < c
toa < b && b < c
. \$\endgroup\$int result = a; if(b < result) result = b; if (c < result) result = c; return result;
? This gives you the same result and has in total as many evaluations as your first if-condition. Similary the "verbose" option of this answer: codereview.stackexchange.com/a/58749/50461 \$\endgroup\$