I would like to give an Integer to whatever demands it. For that I use the following code

public class IntIDGenerator {

private static int sequence = 1;

public synchronized static int generate(){
sequence++;
return sequence;
}

}


It is thread-safe because the synchronized term in the method generate(), but I have the feeling that it is possible to implement this generation in a better way. I was thinking in AtomicInteger, but I will have, more or less, the same problem. Any hint?

-
What do you perceive to be a problem with this implementation, and how is using AtomicInteger susceptible to that same problem? –  bowmore Jun 18 '14 at 15:40

And what's wrong with :

public final class SDGIntIDGenerator {

private static final AtomicInteger = new AtomicInteger(1);

private SDGIntIDGenerator() {}

public static int generate(){
return sequence.getAndIncrement();
}

}


Put private constructor so no instances can be made.
Only static methods means helper class => make class final.

Edit :

While @bowmore is correct you could want more generators for each class, you could do the following :

public final class SDGIntIDGenerator {

private static final ConcurrentHashMap<Class,AtomicInteger> mapper = new ConcurrentHashMap<Class,AtomicInteger>();

private SDGIntIDGenerator () {}

public static int generateId (Class _class) {
mapper.putIfAbsent(_class, new AtomicInteger(1));
return mapper.get(_class).getAndIncrement();
}
}


With this implementation you could ask an id for each class.
This is because the ConcurrentHashMap is also thread safe for some methods.

Important note : Your generator will reset himself after server restart so usage for generating an id for storing in database is a bad idea.

You could also just return the AtomicInteger.
It doesn't matter if you are using a lot of threads and every thread knows that instance of the AtomicInteger.
The AtomicInteger will stay threadsafe.

Footnote : Wrote in java 6, with higher java you could refactor the instantiation of the ConcurrentHashMap.

-
There is no reason to make this a static method. Why can't you have multiple instances, each generating a separate series, for different purposes? –  bowmore Jun 18 '14 at 18:30
@bowmore cause the OP did make the method static, the OP should have his reasons for that. –  chillworld Jun 18 '14 at 18:31
Sure, but that should be part of a review remark. Make it non static, extract interface, allow multiple implementations. –  bowmore Jun 18 '14 at 18:33
@bowmore your Always welcome to write your own review, more answers is more insights. The OP can only be well with that. –  chillworld Jun 18 '14 at 19:36
Returning the AtomicInteger will break encapsulation. Clients could get an id without incrementing, or they could even decrement or increment more than once. –  bowmore Jun 19 '14 at 6:25

protected by MalachiJun 19 '14 at 16:46

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.