AtomicInteger for limited sequence generation

How can we use AtomicInteger for limited sequence generation say the sequence number has to be between 1 to 60. Once the sequence reaches 60 it has to start again from 1. I wrote this code though not开发者_开发问答 quite sure wether this is thread safe or not?

public int getNextValue()
{
 int v;
 do
 {
   v = val.get();
   if ( v == 60)
   {
    val.set(1);
   }
 }
  while (!val.compareAndSet(v , v + 1));
   return v + 1;
  }

As of Java 8, you can use AtomicInteger.updateAndGet:

return val.updateAndGet(n -> (n >= 60) ? 1 : n + 1);

Another alternative would be to simply do...

return val.getAndIncrement() % 60;

...unless you're concerned with exceeding the integer max-value (2147483647). If that is a concern, you could have a look at the getAndIncrement implementation:

public final int getAndIncrement() {
    for (;;) {
        int current = get();
        int next = current + 1;
        if (compareAndSet(current, next))
            return current;
    }
}

All you need to change is the int next... line to something like:

int next = (current + 1) % 60;

Oops. This loops through 0->59. You needed 1->60, so add one to the return-value to get the desired result.

You can do this in a single line using Java 8.

AtomicInteger counter = new AtomicInteger();

public int getNextValue() {
    return counter.updateAndGet(n -> (n >= 60) ? 1 : n + 1);
}

If you make the method synchronized then it will be threadsafe as long as the val is nowhere else accessed. The approach is however a bit cumbersome, I'd rewrite it as follows:

public synchronized int getNextValue() {
    val.compareAndSet(60, 0); // Set to 0 if current value is 60.
    return val.incrementAndGet();
}

This gives 1 until with 60 back inclusive. If you actually need 1 until with 59, then replace 60 by 59.

Any particular reason to use AtomicInteger here rather than just a simple synchronized method?

How about something simple like the following:

private int val=1;

public synchronized int getNextValue() {
 int v=val;
 val = (val==60) ? 1 : (val+1); 
 return v;
}

No, it's not thread safe - you shouldn't call set inside a cycle:

int value, next;
do {
    value = val.get();
    next = (value == 60) ? 1 : (value + 1);
} while (!val.compareAndSet(value, next);
return next;

Quick answer, not thread safe. The test and set need to be atomic, unless you make the whole method synchronized. Note the val.get() and the test of v are not atomic. If the thread yields after v = val.get() you'll get two calls with the same sequence number.

Also, if the compareAndSet fails you never change the values, it'll be an infinite loop.

AtomicInteger has a getAndIncrement() call. That will get you a clean value to return.

The rolling is a bit trickier. One solution is to mod the return value. Something like so:

int v = val.getAndIncrement();
return (v % 60) + 1;

Since each thread has a local copy of v we can safely do some math on it and return the value. There is one sticking point if you get an overflow. Depending on how often you generate a sequence number this may or may not be an issue.