java - Not able to generate a unique user-number

Question

I have a problem when I'm trying to generate a unique customer-id in my application. I want the numbers to start from 1 and go up. I have a register-class using tree-map that generates the next customer-number using this code:

public String generateNumber()
{
    int number = 1;

    for(Map.Entry<String, Forsikringkunde> entry : this.entrySet())
    {
        if(entry.getValue().getNumber().equals(String.valueOf(number)))
        {
            number++;
        } 
    }return String.valueOf(number);
}

When I generate customers in my application I get duplicates of the numbers even though I iterate through the map. When creating a customer I create the object, run this method, use a set-method for the ID and adds it to the register, but it doesn't work. Anyone have a solution?

Answer 1

If you're on Java 8, I suggest you try this:

int max = this.values()
              .stream()
              .map(Forsikringkunde::getNumber)
              .mapToInt(Integer::parseInt)
              .max()
              .orElse(0);

return String.valueOf(max + 1);

Answer 2

Modify the code to instead find the maximum number in your map, and then use that+1:

public String generateNumber()
{
    int max = -1;

    for(Map.Entry<String, Forsikringkunde> entry : this.entrySet())
    {
        int entry = Integer.parseInt(entry.getValue().getNumber());
        if(entry > max)
        {
            max = entry;
        } 
    }
    return String.valueOf(max + 1);
}

(This mimics your coding style. aioobe's answer shows how to do the same thing more elegantly.)

Your method doesn't work because the map is not iterated in order. For example, here's what happens if you iterate through two users with number 2 and 1 respectively:

1. Start with "number = 1"
2. Check if number == 2: it's not, so continue
3. Check if number == 1: it is, so set number = 2

Now the loop is done and number is 2, even though a user with id 2 already exists. If it had been iterated in order, it would have worked.