Reflecting parameter name: abuse of C# lambda expressions or syntax brilliance?

Question

I am looking at the MvcContrib Grid component and I'm fascinated, yet at the same time repulsed, by a syntactic trick used in the Grid syntax:

.Attributes(style => "width:100%")

The syntax above sets the style attribute of the generated HTML to width:100%. Now if you pay attention, 'style' is nowhere specified. It is deduced from the name of the parameter in the expression! I had to dig into this and found where the 'magic' happens:

Hash(params Func<object, TValue>[] hash)
{
    foreach (var func in hash)
    {
        Add(func.Method.GetParameters()[0].Name, func(null));
    }
}

So indeed, the code is using the formal, compile time, name of parameters to create the dictionary of attribute name-value pairs. The resulted syntax construct is very expressive indeed, but at the same time very dangerous.

The general use of lambda expressions allows for replacement of the names used without side effect. I see an example in a book that says collection.ForEach(book => Fire.Burn(book)) I know I can write in my code collection.ForEach(log => Fire.Burn(log)) and it means the same thing. But with the MvcContrib Grid syntax here all of a sudden, I find code that actively looks and makes decisions based on the names I choose for my variables!

So is this common practice with the C# 3.5/4.0 community and the lambda expressions lovers? Or is a rogue one trick maverick I shouldn't worry about?

Solution 1

This has poor interop. For example, consider this C# - F# example

C#:

public class Class1
{
    public static void Foo(Func<object, string> f)
    {
        Console.WriteLine(f.Method.GetParameters()[0].Name);
    }
}

F#:

Class1.Foo(fun yadda -> "hello")

Result:

"arg" is printed (not "yadda").

As a result, library designers should either avoid these kinds of 'abuses', or else at least provide a 'standard' overload (e.g. that takes the string name as an extra parameter) if they want to have good interop across .Net languages.

Solution 2

I find that odd not so much because of the name, but because the lambda is unnecessary; it could use an anonymous-type and be more flexible:

.Attributes(new { style = "width:100%", @class="foo", blip=123 });

This is a pattern used in much of ASP.NET MVC (for example), and has other uses (a caveat, note also Ayende's thoughts if the name is a magic value rather than caller-specific)

Solution 3

Just wanted to throw in my opinion (I'm the author of the MvcContrib grid component).

This is definitely language abuse - no doubt about it. However, I wouldn't really consider it counter intuitive - when you look at a call to Attributes(style => "width:100%", @class => "foo")
I think it's pretty obvious what's going on (It's certainly no worse than the anonymous type approach). From an intellisense perspective, I agree it is pretty opaque.

For those interested, some background info on its use in MvcContrib...

I added this to the grid as a personal preference - I do not like the use of anonymous types as dictionaries (having a parameter that takes "object" is just as opaque as one that takes params Func[]) and the Dictionary collection initializer is rather verbose (I am also not a fan of verbose fluent interfaces, eg having to chain together multiple calls to an Attribute("style", "display:none").Attribute("class", "foo") etc)

If C# had a less verbose syntax for dictionary literals, then I wouldn't have bothered including this syntax in the grid component :)

I also want to point out that the use of this in MvcContrib is completely optional - these are extension methods that wrap overloads that take an IDictionary instead. I think it's important that if you provide a method like this you should also support a more 'normal' approach, eg for interop with other languages.

Also, someone mentioned the 'reflection overhead' and I just wanted to point out that there really isn't much of an overhead with this approach - there is no runtime reflection or expression compilation involved (see http://blog.bittercoder.com/PermaLink,guid,206e64d1-29ae-4362-874b-83f5b103727f.aspx).

Solution 4

I would prefer

Attributes.Add(string name, string value);

It's much more explicit and standard and nothing is being gained by using lambdas.

Solution 5

Welcome To Rails Land :)

There is really nothing wrong with it as long as you know what's going on. (It's when this kind of thing isn't documented well that there is a problem).

The entirety of the Rails framework is built on the idea of convention over configuration. Naming things a certain way keys you into a convention they're using and you get a whole lot of functionality for free. Following the naming convention gets you where you're going faster. The whole thing works brilliantly.

Another place where I've seen a trick like this is in method call assertions in Moq. You pass in a lambda, but the lambda is never executed. They just use the expression to make sure that the method call happened and throw an exception if not.

View more solutions

Comments

[Jamie Penney]

Is it though? html.Attributes.Add("style", "width:100%"); doesn't read as nicely as style = "width:100%" (the actual html generated), whereas style => "width:100%" is very close to what it looks like in the resulting HTML.

[Remus Rusanu]

Their syntax allows for tricks like .Attributes(id=>'foo', @class=>'bar', style=>'width:100%'). The function signature uses the params syntax for variable number of args: Attributes(params Func<object, object>[] args). It is very powerfull, but it took me quite a while to understand wtf it does.

[Jamie Penney]

Something I had not considered. How would one access the expression tree correctly for it to work in the F# case?

[Prakash]

You don't. This strategy is simply non-portable. (In case it helps, as an example the other way, F# could be capable of overloading methods that differ only in return type (type inference can do that). This can be expressed in the CLR. But F# disallows it, mostly because if you did that, those APIs would not be callable from C#.) When it comes to interop, there are always trade-offs at 'edge' features regarding what benefits you get versus what interop you trade away.

[Guffa]

@Jamie: Trying to make the C# code look like the HTML code would be a bad reason for design descisions. They are completely different languages for completely different purposes, and they should not look the same.

[Samantha Branham]

I was a bit hesitant to, but I agree. Aside from the reflection overhead, there is no significant difference between using a string as in Add() vs. using a lambda parameter name. At least that I can think of. You can muck it up and type "sytle" without noticing both ways.

[Funka]

An anonymous object might just as well have been used, without sacrificing the "beauty"? .Attributes(new {id = "foo", @class = "bar", style = "width:100%"}) ??

[Jamie Penney]

That seems like a bit of an oversight on the F# language side -> is there a good reason why this is the case? I'm guessing lambda expressions in C# and functions in F# generate different IL.

[Samantha Branham]

@Guffa Why would it be a bad reason for design decisions? Why should they not look the same? By that reasoning should they intentionally look different? I'm not saying your wrong, I'm just saying you might want to more fully elaborate your point.

[John Farrell]

I don't like non interoperability as a reason to not do something. If interoperability is a requirement then do it, if not, then why worry about it? This is YAGNI IMHO.

[Samantha Branham]

Really? I knew exactly what that snippet of code intended when I looked at it. It's not that obtuse unless you're very strict. One could give the same argument about overloading + for string concatenation, and that we should always use a Concat() method instead.

[Remus Rusanu]

@jfar: In .NET CLR land interoperability has a whole new dimenssion, as assemblies generated in any compiler are supposed to be consumed from any other language.

[Jeffrey Hantin]

@Remus Rusanu: I thought it was okay to do non-interoperable things as long as you weren't claiming to be [CLSCompliant].

[Samantha Branham]

I appreciate compile-time checking, but I think this comes down to a matter of opinion. Maybe something like new Attributes() { Style: "width:100%" } would win more people over to this, since it's more terse. Still, implementing everything HTML allows is a huge job and I can't blame someone for just using strings/lambdas/anonymous classes.

[Guffa]

@Stuart: No, you didn't know exactly, you were just guessing based on the values used. Anyone can make that guess, but guessing is not a good way of understanding code.

[Samantha Branham]

I'm guessing using .Attribute("style", "width:100%") gives me style="width:100%", but for all I know it could give me foooooo. I'm not seeing the difference.

[JMarsch]

I agree that you don't have to be CLS compliant, but it seems like a good idea if you are writing a library or control (and the snippet that start this is from a grid, yes?) Otherwise, you're just limiting your audience/customer base

[Guffa]

@Stuart: Making code that output HTML look like HTML is like making code that prints bar codes look like bar codes... The way code is written should not be influenced by what the output looks like. Making the code intentionally look different doesn't really apply here, but in ASP code I have intentionally exaggerated the differences between VBScript and Javascript to make them easy to recognise.

[Prakash]

This also has interop issues; not all languages support creating an anonymous type like that on the fly.

[Sam Saffron]

I completely agree, but this does not answer the question at all.

[Sam Saffron]

I love how nobody really answered the question, instead people provide a "this is better" argument. :p Is it an abuse or not?

[Metro Smurf]

@Elisha - your Pros and Cons are reversed. At least I hope you're not saying that a Pro is having code "not intuitive". ;-)

[Remus Rusanu]

Is a valid point, but truth in advertising: LINQ comes with a whole set of attributes like TableAttribute and ColumnAttribute that make this a more legit affair. Also linq mapping looks at class names and property names, which are arguably more stable than a parameter names.

[Jamie Penney]

I agree with you there. I've changed my stance on this slightly after reading what Eric Lippert/Anders Helsberg/etc had to say on the matter. Thought I'd leave this answer up as it is still somewhat helpful. For what it's worth, I now think this style of working with HTML is nice, but it doesn't fit with the language.

[Arnis Lapsa]

I would say - it's completely intuitive. :)

[Arnis Lapsa]

For this particular case - lambda parameter name and string parameter are both fragile. It's like using dynamic for xml parsing - it's appropriate because you can't be sure about xml anyway.

[Arnis Lapsa]

It's all about readability. This approach involves more braces and new keyword. Could you add some thoughts why this (using anonymous-type) is more flexible?

[Matt Hinze]

I would be interested to see Eric Lippert's reaction to this. Because it's in FRAMEWORK code. And it's just as horrible.

[Wouter Lievens]

"Guessing based on the values used" is ALWAYS what you do when looking at code. If you encounter a call to stream.close() you assume it closes a stream, yet it might as well do something completely different.

[Seth Petry-Johnson]

So it's a brilliant syntactical abuse of lambda expressions? I think I agree :)

[Guffa]

@Wouter: If you are ALWAYS guessing when reading code, you must have great difficulties reading code... If I see a call to a method named "close" I can gather that the author of the class doesn't know about naming conventions, so I would be very hesitant to take anything at all for granted about what the method does.

[NotDan]

I don't think the problem is readability after the code has been written. I think the real issue is the learnability of the code. What are you going to think when your intellisense says .Attributes(object obj)? You'll have to go read the documentation (which nobody wants to do) because you won't know what to pass to the method. I don't think this is really any better than the example in the question.

[Jeremy Skinner]

I've also tried to address some of the issues raised here in some more depth on my blog: jeremyskinner.co.uk/2009/12/02/lambda-abuse-the-mvccontrib-h‌​ash

[Allyn]

I couldn't figure out why this wasn't weird to me, and then I remembered Rails. :D

[Brad Wilson]

It's no less opaque in Intellisense than anonymous objects.

[davidfowl]

Maybe it's worth changing the: Func<object, string> to Expression<<Func<object, string>> And if you constrain the right hand side of the expression to just be constants you can have an implementation that does this: public static IDictionary<string, string> Hash(params Expression<Func<object, string>>[] hash) { Dictionary<string, string> values = new Dictionary<string,string>(); foreach (var func in hash) { values[func.Parameters[0].Name] = (string)((ConstantExpression)func.Body).Value; } return values; }

[Marc Gravell]

@Arnis - why more flexible: it isn't relying on the implied parameter name, which might (don't quote me) cause issues with some lambda implementations (other languages) - but you can also use regular objects with defined properties. For example, you could have a HtmlAttributes class with the expected attributes (for intellisense), and just ignore those with null values...

[Marc Gravell]

(ah, see the F# example above for an illustration of the lambda implementation wossit - I knew I'd seen it somewhere!)

[Charlie Flowers]

Amen, brother. Amen (2nd amen required to meet min length for comments :)

[Charlie Flowers]

You say it is nothing like Ruby. But it is an awful heck of a lot like Ruby's syntax for specifying the keys and values of a hashtable.

[Phil]

Code that breaks under alpha-conversion! Yey!

[Roatin Marth]

@dfowler: so does func.Parameters[0].Name have better interop support than func.Method.GetParameters()[0].Name? Is that what you're saying?

[Charlie Flowers]

@Guffa, by that reasoning, you'd never use anything like ASP.NET, ASP.NET MVC, JSP, or any similar web technology. The reason they exist is to let your view code look as close as possible to html. We already had servlets before any of that came out, and a servlet is merely a method that generates textual html from code. The world wanted something that looked more like the output that was being generated, and apparently you did too since you mention ASP.

[Jørn Schou-Rode]

+1 for mentioning that the interface is added via optional extension methods. Non C# users (and anyone offended by the language abuse) can simply refrain using it.

[Sam Saffron]

@Charlie, syntactically it looks similar, semantically it is way different.

[Sleeper Smith]

Your comment alone was way more than needed. But then, you can't just Amen once and then put in your comment :D

[andriy]

I like the lambda syntax, but just today I discovered another problem with it. It won't work with the new HTML5 "data-" attributes, because something like data-key isn't a valid C# identifier. Good thing there's the overload to fall back on.

[Guillaume86]

Would be nice to be able to send directly anonymous objects to function(IDictionary x), a bit like the an Expression<Func<>> parameter in a function works for lambdas

[Jordan Wallwork]

Naming a class Controller won't do squat though if you don't inherit from controller too...

[Nicolas Fall]

yeah... that's funny. and maybe it's magical kinda, in the sense of no compile time safety, is there a place where this usage would cause runtime instead of compile-time errors?

[Luaan]

@Kyralessa You could also make an extension method of your own, specifically for the data attributes. Say, .Data(myField => whatever). Now you have something more clear than using Attribute - you are semantically trying to pass data, after all, not an attribute.