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

ActionGroupContext to know an id of the current context

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

Problem

My services(or services that depend on services) need to know about a current action group id. To add an extra optional parameter to each service for the id was an idea but in my current environment to difficult and ugly because every service would have to take in the parameter only to pass it to other services.

My solution was a class ActionGroupContext which stores the current context in a static ThreadStore or HttpStore.

What I don't like especially with my solution is that even though you are not do web application you have to include a reference to System.Web, but the convenience is great, the user does not need any setup code.

Do you have any improvement ideas?

Demo usage:

public void SomeApplicationMehtod()
{
    using (new ActionGroupContext())
    {
        someService.SomeMehtod();
    }
}

public class SomeService
{
    public void SomeMehtod()
    {
        var id = ActionGroupContext.Id;

        //Do something with the Id.

    }
}


The code:

```
public class ActionGroupContext : IDisposable
{
private static readonly GuidStore GuidStore = new GuidStore("Action-Group-Id-Store");

private readonly Guid? oldId;
public ActionGroupContext()
{
oldId = GuidStore.GetGuid();
GuidStore.SetGuid(Id ?? Guid.NewGuid());
}

public static Guid? Id
{
get { return GuidStore.GetGuid(); }
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
if (disposing)
{
GuidStore.SetGuid(oldId);
}
}
}

public class ThreadStore : IStore
{
private readonly LocalDataStoreSlot slot;
public ThreadStore(string storeKey)
{
slot = Thread.GetNamedDataSlot(storeKey);
}

public object GetData()
{
return Thread.GetData(slot);
}

public void SetData(object data)
{
Thread.SetData(slot, data);
}
}

public interface IStor

Solution

Your Dispose pattern is somehow flawed.

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        GuidStore.SetGuid(oldId);
    }
}


You don't check if the object already is disposed, so calling Dispose() over and over will result in each time calling GuidStore.SetGuid(oldId);.

So using a private variable for saving if the object is disposed would be the way to go.

private bool disposed = false;

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (!disposed && disposing)
    {
        GuidStore.SetGuid(oldId);
        disposed = true;
    }
}


Also for completeness you should also use the finalizer like so

~ActionGroupContext()
{
    Dispose(false);
}


One thing what should never happen is that a call to Dispose() throws an exception. If you are 100% sure this can't happen, you are fine. If you aren't 100% sure, you should enclose the call to SetGuid() inside a try..catch block.

Please read this: proper-use-of-the-idisposable-interface

In your posted code I see neither in the ThreadStore nor in the HttpStore any usage of this ActionGroupContext so I see no reason to reference System.Web to use this context.

Code Snippets

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        GuidStore.SetGuid(oldId);
    }
}
private bool disposed = false;

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (!disposed && disposing)
    {
        GuidStore.SetGuid(oldId);
        disposed = true;
    }
}
~ActionGroupContext()
{
    Dispose(false);
}

Context

StackExchange Code Review Q#100483, answer score: 3

Revisions (0)

No revisions yet.