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

Running Code Just Once

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

Problem

I want a code run just once (say in Dispose). WriteOnceBlock from TPL Data Flow could be used here; but again if we need to check if it is done (in a 'not data flow' friendly manner), we have to call Receive and timeout and things.

I wrote this and it works, but I am not sure about when comparison takes place (==); is it possible for that to be rearranged so something goes wrong and out of time/order?

public class Once
{
    const Int32 JobDone = 11011;
    const Int32 NotDone = 119;

    Int32 _done;

    public Once() { _done = NotDone; }

    public bool IsDone() { return Interlocked.CompareExchange(ref _done, JobDone, NotDone) == JobDone; }
}


And usage:

readonly Once _stopped = new Once();
public void OnStop()
{
    if (_stopped.IsDone()) return;

    //...
}

Solution

There are a few items to make this code better.

-
Naming. Once is an OK name for the class, but the method name IsDone is a problem. This is an 'atomic' operation that sets values, as well as gets values. A method called something like "Trigger", and changing the class name to a common term like OneShot, will give you the semantics like:

private readonly OneShot terminator = new OneShot();

if (terminator.Trigger())
{
    ... do something if we are the first trigger
}


or, using your semantics, the negated value:

if (!terminator.Trigger()) return;


-
Your fields should all be private.

private const Int32 JobDone = 11011;
private const Int32 NotDone = 119;

private Int32 _done;


otherwise other code can possibly reset or mess up your trigger.

-
Why use the bizarre numbers for the constants? What's wrong with 1, and 0. The special numbers make me think there's something especially magical with them.

Apart from that, the Interlocked.CompareExchange is the right tool for the job. It creates an atomic compare-and-set operation that makes it thread safe.

Code Snippets

private readonly OneShot terminator = new OneShot();


if (terminator.Trigger())
{
    ... do something if we are the first trigger
}
if (!terminator.Trigger()) return;
private const Int32 JobDone = 11011;
private const Int32 NotDone = 119;

private Int32 _done;

Context

StackExchange Code Review Q#73925, answer score: 10

Revisions (0)

No revisions yet.