views:

105

answers:

2

I want to write an extension method that tests if an attribute is applied on a method call, and I'd like to specify the method as a lambda expression. Currently, I have the following (working) approach, but I really don't like the way this code looks:

// Signature of my extension method:
public static bool HasAttribute<TAttribute, TDelegate>(this Expression<TDelegate> method)

// Usage (current code)
Expression<Func<AccountController, LogInModel, string, ActionResult>> mut = (c, m, s) => c.LogIn(m, s);
mut.HasAttribute<ExportModelStateAttribute, Func<AccountController, LogInModel, string, ActionResult>>().ShouldBeTrue();

As you can see, I have to specify the delegate type twice, and neither time does it look pretty... I'd like to have something more like

// Usage (if I had my way...)
var mut = (c, m, s) => c.LogIn(m, s);
mut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

but I realize that might be asking a tad bit too much.

Is there any way I could refactor away the type arguments from my current code?

The reason I have the TDelegate type argument there in the first place, is that I want to use this regardless of method signature and return type, and depending on the number of input arguments and whether the method in question is a void or a function, TDelegate needs to vary. I don't want to have one different implementation for each number of input arguments to the method I test...

Update:
As Jay points out in a comment, I apparently don't need to specify the TDelegate type argument in the call to HasAttribute<>. The code now looks like this:

Expression<Func<AccountController, LogInModel, string, ActionResult>> mut = (c, m, s) => c.LogIn(m, s);
mut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

It's much better, but I still think the first line is pretty messy. Could it be even better?

+1  A: 

The one thing you can do is replace the long ugly Func<AccountController, LogInModel, string, ActionResult> with

public delegate ActionResult myDelegate(AccountController accountController, LogInModel logInModel, string varString);

Expression<myDelegate> mut = (c, m, s) => c.LogIn(m, s);
mut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

Updated: to show example with the updated question. I do not think it can get much more simplifed.

Scott Chamberlain
That does help to tidy things up a little, but I still have to specify the type of the delegate, even though you've halved the number of times I have to do so. I don't see why C# can't see from the lambda what type it is? After all, the part after `=>` can really only evaluate to one of these...
Tomas Lycken
@Tomas Lambda expressions are used for many things -- `Expression` is just one among them. The compiler can't know that you want an `Expression<Func<T1,T2,TResult>>` as opposed to just a `Func<T1,T2,TResult>`, for example.
Jay
+1  A: 
  1. Can you use LambdaExpression instead of the generic form Expression<>? (if so then the TDelegate param can go away)
  2. If not, would this be ok:

    // Signature of my extension method:
    public static bool HasAttribute<TDelagate>(this Expression<TDelagate> method, 
                                               Type attributeType)
    

The trouble is, you have one type you want to specify explicitly, and another that you want not to (this is impossible in C#).

Regardless of if you can do either of those, you will still need to specify the full delegate type for the expression declaration (at least in C# 3, not sure about C# 4 if that is legal or not, I don't know if there is a way around this or not).

If HasAttribute works the way I think it does, I think you can do #1:

public static bool HasAttribute<TAttribute>(this LambdaExpression method) {
    if (method.Body.NodeType == ExpressionType.Call) {
        MethodCallExpression call = (MethodCallExpression)method.Body;
        return call.Method.GetCustomAttributes(typeof(TAttribute), true).Any();
    }
    return false;
}

Edit:

I think you can simplify the first line with functions like this:

public static LambdaExpression GetMut<T>(Expression<Func<T>> f) { return f; }
public static LambdaExpression GetMut<T>(Expression<Action<T>> f) { return f; }

making the usage:

var l = new LogInModel(); // these variables can be inlined or declared 
var s = "";               // it makes no difference
var mut = Expressions.GetMut<AccountController>(c => c.LogIn(l,s));
mut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

full code for console app used to toy with this:

class Program {
    static void Main(string[] args) {
        var mut = Expressions.GetMut<AccountController>(c => c.LogIn(new LogInModel(), ""));
        mut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

        var failmut = Expressions.GetMut<AccountController>(c => c.LogInFails());
        failmut.HasAttribute<ExportModelStateAttribute>().ShouldBeTrue();

        Console.ReadKey();
    }
}

public class ExportModelStateAttribute : Attribute { }

public class ActionResult { }

public class LogInModel { }

public class AccountController {
    [ExportModelState]
    public ActionResult LogIn(LogInModel model, string s) {
        return new ActionResult();
    }

    public void LogInFails() {}
}

public static class Expressions {
    // only need this to find the method given the class T
    public static LambdaExpression GetMut<T>(Expression<Action<T>> func) { return func; }
    // Signature of my extension method:
    public static bool HasAttribute<TAttribute>(this LambdaExpression method) {
        if (method.Body.NodeType == ExpressionType.Call) {
            MethodCallExpression call = (MethodCallExpression)method.Body;
            return call.Method.GetCustomAttributes(typeof(TAttribute), true).Any();
        }
        return false;
    }

    public static void ShouldBeTrue(this bool obj) {
        Console.WriteLine(obj);
    }
}
Bill Barry
This approach looks promising. How would the usage lines look if I typed the method expression as a `LambdaExpression`?
Tomas Lycken
Exactly like it does in your update.
Bill Barry
@your edit: But doesn't this still require me to implement a new `GetMut`-method (Mut for **m**ember **u**nder **t**est, btw...) for each number of parameters I need in the expression? That is, one for `Func<T>`, one for `Func<T1, T2>` etc...?
Tomas Lycken
I don't think so. I am not sure why it works, but apparently it does (for any number of params). Actually you apparently don't even need the `GetMut<T>(Expression<Func<T>> func)` for any of these exceptions. edit: heh it just clicked... this creates an action who's first parameter is the `T` type, ignoring whatever is returned. Updating the answer now...
Bill Barry