HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Thread-safe lock-free counter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
freethreadsafecounterlock

Problem

I was using such code:

class Counter
{
    private int i = 0;

    public int Next()
    {
        lock (this)
        {
            return i++;
        }
    }

    public void Reset()
    {
        lock (this)
        {
            i = 0;
        }
    }

}


I refactored it such a way:

class Counter
{
    private int i = 0;

    public int Next()
    {
        return Interlocked.Increment(ref i);
    }

    public void Reset()
    {
        i = 0;
    }

}


Will that work?

I'm using .NET 4.5 if this matters.

Solution

It should also be fine in earlier versions of .NET as you are not using any 4.5 specific classes or language features.

The reset is fine as the assignment is atomic as you mentioned, however if you were using a long then you would want to use Interlocked.Exchange because the assignment of a 64bit number is not atomic on a 32bit operating system (as it is stored as 2 32bit values).

The only changes I would suggest are purely from a coding standards perspective:

// seal the class as it's not designed to be inherited from.
public sealed class Counter
{
    // use a meaningful name, 'i' by convention should only be used in a for loop.
    private int current = 0;

    // update the method name to imply that it returns something.
    public int NextValue()
    {
        // prefix fields with 'this'
        return Interlocked.Increment(ref this.current);
    }

    public void Reset()
    {
        this.current = 0;
    }
}

Code Snippets

// seal the class as it's not designed to be inherited from.
public sealed class Counter
{
    // use a meaningful name, 'i' by convention should only be used in a for loop.
    private int current = 0;

    // update the method name to imply that it returns something.
    public int NextValue()
    {
        // prefix fields with 'this'
        return Interlocked.Increment(ref this.current);
    }

    public void Reset()
    {
        this.current = 0;
    }
}

Context

StackExchange Code Review Q#15121, answer score: 9

Revisions (0)

No revisions yet.