Attribute driven behaviour in C# methods

Question

We want to create a TransactionScope factory class that we can use as a central point for instantiating TransactionScopes with varying configurations throughout our app.

One requirement we have is that a method can either:

1. Instantiate a plain TransactionScope whose settings are driven by the defaults in the App.Config
2. Instantiate a TransactionScope passing some config key, which will pull specific settings from some other source

The latter requirement is so that settings can be changed at runtime for specific methods if necessary (e.g. extending a timeout) without having to recompile the system and without affecting all TransactionScopes.

Option 1 - pass config keys via method param on the create methods

public static class TransactionScopeFactory
{
    public static TransactionScope Create()
    {
        return new TransactionScope();
    }

    public static TransactionScope Create(string configKey)
    {
        var source = GetConfigSettings(configKey);
        if(source != null)
        {
            var options = new TransactionOptions
                              {
                                  //IsolationLevel = From Config Source
                                  //Timeout = From Config Source
                              };
            return new TransactionScope(TransactionScopeOption.Required, options);
        }
        return Create();
    }
}

public class Frob
{
    public void DoStuff()
    {
        using (var scope = TransactionScopeFactory.Create()) //Default
        { /*Do Stuff*/ }
    }
    public void DoFoo()
    {
        using (var scope = TransactionScopeFactory.Create("DoFoo"))
        { /*Do Foo*/ }
    }
    public void DoBar()
    {
        using (var scope = TransactionScopeFactory.Create("DoBar"))
        { /*Do Bar*/ }
    }
}

My only issue with this is that I don't really like the fact that there are different strings peppered throughout the different Create() methods throughout the app. Even moving them to a utility class as const strings makes the code feel less "clean".

I was thinking about another way to do this with MethodAttributes. Instead, the TransactionScopeManager would pull the correct configuration key from a MethodAttribute using reflection and so the actual Create() would only ever be that plain parameter less value.

Option 2 - Pass config keys via method param on the create methods

[AttributeUsage(AttributeTargets.Method, Inherited = false, AllowMultiple = false)]
public class TransactionScopeConfigurationAttribute : Attribute
{
    public string ConfigKey { get; set; }
}

public static class TransactionScopeFactory
{
    public static TransactionScope Create()
    {
        string configKey = GetConfigKeyByReflection();
        var source = GetConfigSettings(configKey);
        if(source != null)
        {
            var options = new TransactionOptions
                              {
                                  //IsolationLevel = From Config Source
                                  //Timeout = From Config Source
                              };
            return new TransactionScope(TransactionScopeOption.Required, options);
        }
        return Create();
    }

    private static string GetConfigKeyByReflection()
    {
        var attributes = (from frame in (new StackTrace()).GetFrames()
                          let attribs = frame.GetMethod().GetCustomAttributes(true).ToList()
                          select attribs).SelectMany(a => a.ToList());


        var attrib = attributes.FirstOrDefault(a => a.GetType() == typeof(TransactionScopeConfigurationAttribute));

        return (attrib != null)
                   ? (attrib as TransactionScopeConfigurationAttribute).ConfigKey
                   : null;
    }
}

public class Frob
{
    //Default - No Attribute
    public void DoStuff()
    {
        using (var scope = TransactionScopeFactory.Create()) 
        { /*Do Stuff*/ }
    }

    [TransactionScopeConfiguration(ConfigKey = "DoFoo")]
    public void DoFoo()
    {
        using (var scope = TransactionScopeFactory.Create())
        { /*Do Foo*/ }
    }

    [TransactionScopeConfiguration(ConfigKey = "DoBar")]
    public void DoBar()
    {
        using (var scope = TransactionScopeFactory.Create())
        { /*Do Bar*/ }
    }
}

Now TransactionScopeManager only has a single solitary method Create(). Overriding what configuration it should use is now done via a setting on an attribute. I'm kinda torn over this implementation. On the face of it, it seems slightly more elegant and makes the code look a little neater... but at the same time, the behaviour isn't as discoverable (i.e. parameter listing in the IntelliSense popup is pretty obvious, having to use an attribute is not so much).

Also, it introduces reflection into the mix which I'm sure is going to cause a performance hit having to generate and walk those StackTrace/Frames every time I need a new TransactionScope.

Answer 1

Eoin, nice job with the second attempt. However, I would stick to your guns on the first style with a few deviations. Either A) pass in enumerations into the create method or B) create an override method with the specific name.

I would highly recommend direction B. This prevents magic strings all over the place and reflection while promoting readability.

Example:

private static TransactionScopeFactory Create(string key){/* primary create */}

public static TransactionScopeFactory Start(){return Create("default_key");}

public static TransactionScopeFactory StartFoo(){return Create("foo");}

public static TransactionScopeFactory StartBar(){return Create("bar");}

Answer 2

Getting information from the stacktrace is so incredibly hacky and nasty. This is not guaranteed to work. If some of your methods get inlined, the stack frame is gone.

This is a completely counter-intuitive method to do this. Someone, who doesn't know, will make a refactoring and the code starts to break. Nobody knows why there is no transaction anymore.

In general, you should make important things explicit, so it is hard to inadvertently screw them up.