patterncsharpMinor
Function that tests and mutates
Viewed 0 times
functionteststhatandmutates
Problem
I am a huge proponent of self-documenting code (when at all possible). I prefer to have code be self evident by the very nature of the names of the variables, methods, etc.
For instance:
I have run into a situation that has defied all attempts to name the method in a way that illustrates what it does. The problem is that the method is required to do two things.
I have a class that must maintain its own state, however it must have a method that both verifies a condition, and changes the state via a thread safe lock to prevent race conditions.
It basically looks like the code below:
As you can see, I have to perform the test (to retrieve the Boolean) and modify state at the same time. If I didn't do this, then a race condition is possible where the value can be changed by another thread in between checking it's value and mutating its state.
The naming
For instance:
if (TheSky.IsBlue && TheGrass.IsGreen)
BeHappy();I have run into a situation that has defied all attempts to name the method in a way that illustrates what it does. The problem is that the method is required to do two things.
I have a class that must maintain its own state, however it must have a method that both verifies a condition, and changes the state via a thread safe lock to prevent race conditions.
It basically looks like the code below:
public class TimeWindow
{
private static object lockObject = new object();
private DateTime Timestamp;
private int WindowCounter;
// What do I call this? CanWeDoSomething() implies that it is merely
// calculating a true/false state, not mutating state.
//
// I don't want to call it
// CanWeDoSomethingAndDecrementItIfWeActuallyDoIt() as that is
// just too annoying, and it messes up the self-documenting nature
// since if (CanWeDoSomethingAndDecrementItIfWeAcutallyDoIt()) describes
// two actions, but only one of them is a Boolean condition, so it doesn't
// really fit in with the if statement.
public bool CanWeDoSomething(int WindowSeconds) {
lock(lockObject) {
if ((DateTime.Now - TimeStamp).TotalSeconds > WindowSeconds) {
Timestamp = DateTime.Now;
WindowCounter = 0;
}
if (WindowsCounter < 10) {
++WindowCounter;
return true;
}
return false;
}
}
}As you can see, I have to perform the test (to retrieve the Boolean) and modify state at the same time. If I didn't do this, then a race condition is possible where the value can be changed by another thread in between checking it's value and mutating its state.
The naming
Solution
I would call this method
As a side note - you can rewrite the code without locks in case if that's the only place you use the
AcquireTimeSlot or similar, since the main action here is actually capturing a timely-limited resource. Result of the method specifies whether resource has been acquired or not. You may also call it TryAcquireTimeSlot to highlight the fact that acquisition may fail.As a side note - you can rewrite the code without locks in case if that's the only place you use the
lockObject. It will be a bit more verbose though...public class TimeWindow
{
private static readonly object _lockObject = new object();
private long _timestampTicks;
private int _windowCounter;
private void CheckWindowExpiration(int windowSeconds)
{
long timestampTicks = _timestampTicks;
while (TimeSpan.FromTicks(DateTime.Now.Ticks - timestampTicks).TotalSeconds > windowSeconds)
{
if (Interlocked.CompareExchange(ref _timestampTicks, DateTime.Now.Ticks, timestampTicks) == timestampTicks)
{
_windowCounter = 0;
break;
}
timestampTicks = _timestampTicks;
}
}
public bool TryAcquireTimeSlot(int windowSeconds)
{
CheckWindowExpiration(windowSeconds);
int windowCounter = _windowCounter;
while (windowCounter < 10)
{
if (Interlocked.CompareExchange(ref _windowCounter, windowCounter + 1, windowCounter) == windowCounter)
return true;
windowCounter = _windowCounter;
}
return false;
}
}Code Snippets
public class TimeWindow
{
private static readonly object _lockObject = new object();
private long _timestampTicks;
private int _windowCounter;
private void CheckWindowExpiration(int windowSeconds)
{
long timestampTicks = _timestampTicks;
while (TimeSpan.FromTicks(DateTime.Now.Ticks - timestampTicks).TotalSeconds > windowSeconds)
{
if (Interlocked.CompareExchange(ref _timestampTicks, DateTime.Now.Ticks, timestampTicks) == timestampTicks)
{
_windowCounter = 0;
break;
}
timestampTicks = _timestampTicks;
}
}
public bool TryAcquireTimeSlot(int windowSeconds)
{
CheckWindowExpiration(windowSeconds);
int windowCounter = _windowCounter;
while (windowCounter < 10)
{
if (Interlocked.CompareExchange(ref _windowCounter, windowCounter + 1, windowCounter) == windowCounter)
return true;
windowCounter = _windowCounter;
}
return false;
}
}Context
StackExchange Code Review Q#26407, answer score: 4
Revisions (0)
No revisions yet.