More bugs
I ran into another sweet little bug that I thought I would share with you. This one cropped up after it passed a few tests. See if you can spot the problem. Without further delay here it is
Problem:
A record needs to be inserted with a new identity number. This number is obtained as the next maximum ID value from a set of elements in a XML file. The following code is supposed to obtain the next ID from the XML. Lets skip the XML related code since that just adds clutter.
Code:
int max = 0; for( DataType data : someListFromTheXml ) { if(max < data.getId()) { max = data.getId()+1; } } s_logger.info(" Max id: " + max); return max;
See anything wrong with it ? Its got a pretty big flaw. The logic works only for odd number of elements. When the number of elements is even, it fails. Here’s an illustration
Test cases:
public class Test { public static void main(String... args) { int [] input1 = new int[]{1}; int [] input2 = new int[]{1,2}; int [] input3 = new int[]{1,2,3}; int [] input4 = new int[]{1,2,3,4}; Test test = new Test(); test.go(input1); test.go(input2); test.go(input3); test.go(input4); test.go(input5); } public void go(int [] input) { int max=0; for(int id:input) { if(max < id) { max = id+1; } } System.out.println(" Max id: " + max); } }
Here’s the output:
Max id: 2
Max id: 2
Max id: 4
Max id: 4
Max id: 6
The logic incorrectly assigns the max to id+1. Instead it should have stored the current max and returned max+1 at the end of the loop. Sigh.
In hind sight a Xpath expression would have done nicely here. Something like max(/Parent/child/@value)+1. Still that is no excuse for the faulty logic 
or you could just change your if condition to if(max <= id)
should you use <= inside < ?
It looks like you make the assumption that the id can not be zero. A list with just one element having id zero will also fail.
Two things to change:
1) start with max = SMALLEST_INT
2) change the conditional to if ! (max > current)
@Manuel Iglesias
@slim ouertani
Yep thats an option.
@Steve
Yep that condition would fail too. The max = max + 1 statement would still need to be fixed.
Fortunately I am not the owner of this code
This makes more sense:
public void go(int [] input)
{
int max=0;
for(int id:input)
{
if(max < id)
{
max = id;
}
}
int next = max +1;
System.out.println("Next id: " + next);
}
@None
Yep that would work. However as Steve pointed out, the assumption would be that IDs cannot start with 0.
In our case, the IDs indeed did not start from 0. They started from 1.
public void go( int[] input )
{
int next = 0; // default ID;
if( input.length() > 0 ) { // check that we don’t have an empty array
next = input[0] + 1; // use the first ID as a base line
for( int i = 1; i < input.length(); ++i ) { // scan the rest
if ( next <= input[i] ) {
next = input[i] + 1;
}
}
}
System.out.println( "Next id: " + next );
}