# Find min of 3 numbers hardcoded

public static int min(int a, int b, int c)
{
int result = 0 ;
if( a < b && a < c && b < c) result = a ;
else if( a < b && a < c && b > c) result = a ;

else if( a > b && a < c && b < c) result = b ;
else if( a < b && b  > c &&  c < a) result = c ;

else if( a > b && b < c && a > c) result = b ;
else if( a > b && a > c && c < b) result = c ;
return result ;
}


Is it better than nested if statements? Is there a more readable solution than this? To me it looks pretty readable, but I'm not sure whether it can be improved.

• If if and and and look readable to you, try reading the code outloud. In before buffalo buffalo buffalo and james while john. – corsiKa Aug 1 '14 at 14:57
• Just something minor but you can simplify this a < b && a < c && b < c to a < b && b < c. – But I'm Not A Wrapper Class Aug 1 '14 at 15:13
• It's hard to evaluate this without more context. Is there a reason you're avoiding Math.Min? Are you trying to optimize the number of comparisons? Are you sure you won't need to handle 4 inputs? – Pierre Menard Aug 1 '14 at 17:11
• your condidions are pretty complex. Why not do something like 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 – Mark Aug 4 '14 at 7:53
• you're wasting time calculating an expression again and again if the previous conditions are false – phuclv Aug 4 '14 at 8:05

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.

• Personally, if I went your way, I would just return c after finding that neither a nor b is the correct answer. – Pimgd Aug 1 '14 at 13:07
• @Pimgd Yes, that'd also be a satisfactory solution. But when using an exception, you are really sure that c really has to be the smallest. – Simon Forsberg Aug 1 '14 at 13:13
• @Pimgd No, I don't know of such a situation, but if such a situation exists (highly unlikely) it will throw an Exception instead of incorrectly returning 3. – Simon Forsberg Aug 1 '14 at 13:19
• Oh wait. int.... leave it as it is then, but note that NaNs can happen with floating point numbers... – example Aug 3 '14 at 2:52
• If we get past if (a <= b && a <= c) return a; without returning a, we can safely assert that a > min(a, b, c), or equivalently, min(b, c) == min(a, b, c). Therefore, we can simplify if (b <= a && b <= c) return b; to if (b <= c) return b;, or even do away with return c; and just do return b <= c ? b : c; – bcrist Aug 3 '14 at 10:48

For these things we have java.lang.Math:

public static int min(final int a, final int b, final int c){
return Math.min(a, Math.min(b, c));
}


Wow, look how short it is!

But it's 3 numbers today, it's 10 tomorrow.
As an alternative, how about an array?

public static int min(int... numbers){
if (numbers.length == 0){
throw new IllegalArgumentException("Can't determine smallest element in an empty set");
}
int smallest = numbers;
for (int i = 1; i < numbers.length; i++){
smallest = Math.min(smallest, numbers[i]);
}
return smallest;
}


I'd use the java.lang.Math solution, it's very short, and very readable.

• For your 1st case there is java.util.Collections.min(). However, it works on collections. – Kao Aug 1 '14 at 12:59
• I actually went looking for that, but then I realized I was using an array. Yet another possibility to use! It uses a Comparator though, so even with Integer objects you end up writing some wrapper. – Pimgd Aug 1 '14 at 13:00
• Your second example (with the array) seems more complicated than what I have (see my answer). – Ryan Aug 1 '14 at 20:47
• @Ryan That's because it's $O(n)$ and not $O(2n)$. It's faster because it doesn't first cast it into a list. Additionally, it doesn't autobox and unbox the int's, so that's another performance issue mitigated. – Pimgd Aug 1 '14 at 20:49
• @Pimgd While you are correct that it has less performance, I think that readability and maintainability should be taken into account. – Ryan Aug 1 '14 at 22:21

I'd suggest using ternary operator, which is quite readable for me:

public static int min(int a, int b, int c) {
return (a < b) ? (a < c) ? a
: c
: (b < c) ? b
: c;
}


However, if you prefer ifs:

public static int min(int a, int b, int c) {
int min = a;
if (min > b) min = b;
if (min > c) min = c;
return min;
}


In both cases only two comparisons will be performed.

• I disagree, that ternary operator is not readable. Your other method is perfectly fine though. – Simon Forsberg Aug 1 '14 at 13:11
• @Kao Perhaps, but having even a small amount of unreadable/unmaintainable code in an application only spawns more of the same. I'd prefer to keep my code clean, easily maintainable and very readable. Even if more readable code leads to a small performance drop, I'll gladly take that hit since it leads to more robust code. – FreeAsInBeer Aug 1 '14 at 14:16
• @KyleHale actually for the scope of the problem I definitely agree with FreeAsInBeer. A ternary itself is already the less readable approach, and then unintuitively creating additional confusion like in the first, by nesting them, enrages me... – Vogel612 Aug 1 '14 at 14:38
• I've taken the liberty of reformatting the ternary expression to make it slightly more readable, though I still find the if-if version easier to understand. – 200_success Aug 1 '14 at 18:41
• The second is also unreadable. I have to think really hard when you use > in a min operation. – djechlin Aug 4 '14 at 1:53

Here is my idea: use an optional parameter, and use built-in libraries. Much clearer and simpler to understand. This works with 3 parameters, but also works with any # of parameters.

public static Integer min(Integer... numbers) {
if (numbers.length == 0) return 0;
// wanted this: assert numbers.length > 0;, but does not work
return Collections.min(Arrays.asList(numbers));
}


Calling with:

System.out.println(min(2,1,3));


gives 1.

I tried to find a solution using generics but cannot get it to work.

Edit: Use IllegalArgumentException (thanks @SimonAndréForsberg!):

public static Integer min(Integer... numbers) {
if (numbers.length == 0) throw new IllegalArgumentException("Cannot have 0 arguments, i.e. min()");
return Collections.min(Arrays.asList(numbers));
}


Edit: The reason this solution works is that Integer... numbers allows for the program calling it to specify any number of arguments (even 0), and inside min here, it is treated as an array, which we can find the minimum of that array using Collections.min and Arrays.asList.

I don't think it's really readable, as it conveys a lot of logic, which is simply unneeded.

Also you should, according to most of the common Java coding standards, not be placing { on its newline, also there should be no blank space between result and ; in return result ;

I would suggest following, though it needs Java 8:

public static int min(final int first, final int... others) {
return IntStream.concat(IntStream.of(first), IntStream.of(others))
.min()
.getAsInt();
}


This way you cover the following:

• You allow an arbitrary amount of integers to be passed to the method.
• You get the min() in a clear way, using Java 8 streams, it checks the minimum somehow.
• Because you know you have at least one result, you can safety get the value from the OptionalInt.

One minor nitpick is that the IntStream.concat(IntStream.of(first), IntStream.of(others)) to construct an IntStream is rather ugly.

• Where do you get the coding standards from to say the braces should be on the same line as the method signature? – Pimgd Aug 1 '14 at 19:49
• @Pimgd - I need to collect the three typical code-style guides in to one place, probably the Java tag wiki – rolfl Aug 1 '14 at 19:54