patterncsharpMinor
Visitor Pattern/Leaky Bucket variant implementation to run an operation at a certain interval
Viewed 0 times
variantintervaloperationvisitorbucketleakyimplementationcertainpatternrun
Problem
My code is a variant on the Visitor Pattern and a "leaky bucket" variant. The goal is pretty straightforward: the "bucket" will collect a specified number of items (say, for example, 500) and then empty the queue, running some CPU-bound operation on ("visiting") each item. (Yes, I'm aware that there's probably existing code "out there" that does something similar to what I'm doing here - this code really isn't all that novel of an idea).
I've read documentation on how to implement the Visitor pattern before but this is truthfully the first time I've actually implemented one. Is what I have below a "valid" way to implement this? Also, have I properly implemented the concurrency/thread safety? I think it should be free from race conditions but it's also possible that I can implement this more simply.
```
///
/// "Overflowing bucket" implementation
///
/// Type of the data structure we're operating on
/// Type of the items in the data structure
///
/// The basic idea of this data structure is that it'll collect a certain number of items and then empty the queue.
///
/// The "overflowing bucket" metaphor isn't perfect because every time that the "bucket" is "filled to the brim" or starts
/// to "overflow" we just empty the whole thing.
///
public class OverflowingBucket
{
#region Fields
// Not volatile - this will only be accessed by the background thread
private readonly T itemToActOn;
// Not volatile - this will only be accessed by the background thread
private readonly Action visitorOperation;
// This action runs after we empty the queue
private readonly Action afterAction;
// Must be concurrent because we could add to the queue while an operation is in place
private readonly ConcurrentQueue queue;
// Will be accessed by multiple threads
private volatile bool inProgress = false;
// Obviously used for locking
private readonly object lockObj = new object();
#endregion Fields
#region Constructor
I've read documentation on how to implement the Visitor pattern before but this is truthfully the first time I've actually implemented one. Is what I have below a "valid" way to implement this? Also, have I properly implemented the concurrency/thread safety? I think it should be free from race conditions but it's also possible that I can implement this more simply.
```
///
/// "Overflowing bucket" implementation
///
/// Type of the data structure we're operating on
/// Type of the items in the data structure
///
/// The basic idea of this data structure is that it'll collect a certain number of items and then empty the queue.
///
/// The "overflowing bucket" metaphor isn't perfect because every time that the "bucket" is "filled to the brim" or starts
/// to "overflow" we just empty the whole thing.
///
public class OverflowingBucket
{
#region Fields
// Not volatile - this will only be accessed by the background thread
private readonly T itemToActOn;
// Not volatile - this will only be accessed by the background thread
private readonly Action visitorOperation;
// This action runs after we empty the queue
private readonly Action afterAction;
// Must be concurrent because we could add to the queue while an operation is in place
private readonly ConcurrentQueue queue;
// Will be accessed by multiple threads
private volatile bool inProgress = false;
// Obviously used for locking
private readonly object lockObj = new object();
#endregion Fields
#region Constructor
Solution
First of all, IMHO the code is well written and has some meaningful comments.
Visitor Pattern
Actually, I am not sure if the
The
OverflowingBucket
to
TestOverflowingBucket
Visitor Pattern
Actually, I am not sure if the
OverflowingBucket has something to do with the visitor pattern. In my eyes, the visitor pattern can be used to add logic to a data structure by implementing visitors. A visitor visits each element of the data structure and decide what to do depending of the element's type.The
OverflowingBucket can be used to add logic to an object for processing other items in a special manner. Therfore I wouldn't use the term "visitor pattern" here.OverflowingBucket
- Because it is not a visitor in my eyes, I would rename the method
Visit
to
Process or something like that.- Consider to make the method
Processprivate and add another methodProcessPendingItemsor something like that.
- The only case where the method
Visitreturns false is, ifqueue.Any()is true andqueue.TryDequeue(out item)is false. That should never happen in the current version of the class... therefore I would make the method void.
TestOverflowingBucket
- The delay does not delay the Add-Task but the main thread. Adding
Thread.Sleep()within the operation delegate would be more realistic.
- There is no need to check if the task is completed. You can just add them all to the
taskslist.
Context
StackExchange Code Review Q#142629, answer score: 6
Revisions (0)
No revisions yet.