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?

Not sure, but not every map saves the order in which you put elements into it. Maybe the map iterates several times over the same object? - See http://stackoverflow.com/questions/2889777/difference-between-hashmap-linkedhashmap-and-treemap — hamena314, May 08 '15 at 19:46
Ah, my bad. It's supposed to be number. Just forgot to translate that part of the code. — ssandoy, May 08 '15 at 19:48

score 1 · Answer 1 · answered May 08 '15 at 19:54

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);

that other guy · Accepted Answer · 2015-05-08T19:57:55.507

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:

Start with "number = 1"
Check if number == 2: it's not, so continue
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.

Thank you so much! That did the trick. Thought that was what I was doing in the original code.... — ssandoy, May 08 '15 at 20:00

Not able to generate a unique user-number

2 Answers2