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

M

21

433

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?

Macguiness answered 11/11, 2009 at 21:0 Comment(26)

+1 for bringing the hashrocket to C# – Node 11/11, 2009 at 21:7

change foreach (var func in hash) to: Array.ForEarch(hash, func => Add(func.Method.GetParameters()[0].Name, func(null))); – Pinero 11/11, 2009 at 21:9

I would argue that this looks obvious, as long as you are willing to look at the intention of the code, rather than merely parsing syntax. Which, if you are reading good code, is what you should be doing anyway. Syntax is just a vehicle for intention, and I would argue that this is intention revealing code. – Kraul 11/11, 2009 at 21:11

Were they former Perl users? There, style => "width:100%" is equivalent to "style", "width:100%" in every way. – Jugendstil 11/11, 2009 at 21:12

@Eric It has gotta be bad when part of the compiler team makes a comment like that. Maybe ask Anders what he thinks? – Kraul 11/11, 2009 at 21:29

I'm waiting for stackoverflow.com/users/8560 to step in and defend his code... – Macguiness 11/11, 2009 at 21:41

This is just asking for trouble and unintended consequences. – Jaxartes 11/11, 2009 at 22:23

I just asked Anders (and the rest of the design team) what they thought. Let's just say the results would not be printable in a family-friendly newspaper. – Apperceive 11/11, 2009 at 22:37

@Eric Lippert: There are baudy saucy raunchy blue off color vile non family friendly newspapers? Where can I find these? In addition could you elaborate on the specifics of the horridness? I'd really like to hear why you guys think it's so bad... – Attar 11/11, 2009 at 22:56

Sure. Seattle's is "The Stranger". As for why this is horrid, we could start with unobvious, clever (remember, clever is bad, clever code is hard to maintain), not at all within the by-design use cases envisaged by the designers of lambdas, slow, brittle, unportable, and unnecessary. – Apperceive 11/11, 2009 at 23:6

C# is currently missing a clean, light syntax for maps, especially maps passed in to functions. Various dynamic languages (Ruby, Python, JavaScript, ColdFusion 9) have a clean, light syntax for that, to some extend or another. – Eisinger 12/11, 2009 at 0:10

@Eric Thanks for asking. I have argued below that this is a nice looking syntax, but can understand your argument against it. Those are all valid reasons not to do something. – Kraul 12/11, 2009 at 0:19

Oh, I think it LOOKS delightful. As for a syntax for maps, yeah, it would be great if we could do new { {"Do", "A deer" } , {"Re", "golden sun"} ... } and have the compiler infer the construction of a map, just as new[] {1, 2,3 } infers the construction of an int array. We're considering these sorts of things. – Apperceive 12/11, 2009 at 1:49

What's a syntax map btw? – Guntar 12/11, 2009 at 19:25

A "map" is what the .NET library calls a "dictionary" -- a device which "maps" a key (and the key can be one key, a pair of keys, whatever) onto a value. A great many concepts in programming are actually just a fancy way of doing a map. For example, you could think of someObject.SomeProperty as being a map from the pair (value of someObject, name of SomeProperty) to the value of the property. Because this idea is so fundamental, it is nice when a programming language provides a slick syntax for describing an arbitrary map. Collection initializers are a good start. – Apperceive 12/11, 2009 at 21:27

Just blogged by K. Scott Allen - "Your Abomination Is My Clever Hack" odetocode.com/Blogs/scott/archive/2009/11/30/… – Louisville 2/12, 2009 at 12:27

So I guess this is what happens when someone wants to use Rails but knows C#? Not everything you can do in RoR needs to be ported to other languages and frameworks – Lantern 2/12, 2009 at 13:45

I've tried to respond to some of the issues raised here on my blog: jeremyskinner.co.uk/2009/12/02/lambda-abuse-the-mvccontrib-hash – Evvoia 2/12, 2009 at 20:58

Lambdas are going to become the regular expressions of .NET shrug – Monaghan 7/12, 2009 at 11:51

Eric Lippert, I respect you greatly, but I think you and the group (including Anders, who I respect FREAKIN' TREMENDOUSLY) are slamming this too harshly. As you admit, C# lacks a tight syntax for maps, and some other langs (like Ruby) have great ones. This guy found a way to get the syntax he wanted. I'll grant you there are similar ways to express it that are almost as expressive as his syntax with fewer downsides. But his syntax and the fact he worked so hard to get it clearly shows a need for a language enhancement. Past performance indicates you guys will create something great for it. – Xyloid 8/1, 2010 at 6:14

I wonder what happens when this is optimized a bit too much, or worse - obfuscated. – Daph 14/11, 2010 at 19:23

I wouldn't hire anyone who either does this or doesn't immediately understand why it shouldn't be done. – Hexastyle 11/5, 2011 at 5:13

I love C#, but it isn't the most expressive a language could possibly be. There are plenty of meta-programming concepts that C# can't express - yet we come upon problems whose solutions require or could greatly benefit from them. The CLR hides all kinds of magical abuses of the lower-levels, and some people complain about it too. But look what it enables us to do! Same with this lambda trick. It's a well rounded solution to an otherwise ugly C# expression, it seems well documented and it is obvious in it's usage/purpose visually, and it helps us create cool websites quickly. – Woollyheaded 22/6, 2011 at 15:3

the anonymous type to IDictionary<string,object> would be a better choice IMHO. And it's used in a lot of places in asp.net MVC. A map type with simple init syntax would be very nice (similar syntax than anonymous types?) edit: didn't seen the Marc Gravell answer... – Poree 7/7, 2011 at 9:13

who is contributing and contributor here? :) – Sericeous 2/12, 2013 at 8:20

this is disqualified by the fact alone, that a valid html attribute name is not necessarily a valid c# identifier name, think of reserved keywords. – Chukar 28/1, 2018 at 15:1

M

149

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.

Mold answered 11/11, 2009 at 21:10 Comment(11)

Something I had not considered. How would one access the expression tree correctly for it to work in the F# case? – Kraul 11/11, 2009 at 21:13

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. – Mold 11/11, 2009 at 21:17

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. – Kraul 11/11, 2009 at 21:26

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. – Dannica 11/11, 2009 at 21:30

@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. – Macguiness 11/11, 2009 at 21:36

@Remus Rusanu: I thought it was okay to do non-interoperable things as long as you weren't claiming to be [CLSCompliant]. – Punctuate 11/11, 2009 at 21:40

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 – Gentleness 11/11, 2009 at 22:15

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; } – Govern 3/12, 2009 at 11:19

@dfowler: so does func.Parameters[0].Name have better interop support than func.Method.GetParameters()[0].Name? Is that what you're saying? – Gerta 22/1, 2010 at 16:8

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. – Trousers 12/5, 2011 at 23:56

@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. – Slob 18/8, 2015 at 8:32

H

153

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)

Hunchback answered 11/11, 2009 at 21:35 Comment(8)

This also has interop issues; not all languages support creating an anonymous type like that on the fly. – Mold 11/11, 2009 at 23:17

I love how nobody really answered the question, instead people provide a "this is better" argument. :p Is it an abuse or not? – Dorking 12/11, 2009 at 0:12

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? – Veratridine 2/12, 2009 at 8:8

I would be interested to see Eric Lippert's reaction to this. Because it's in FRAMEWORK code. And it's just as horrible. – Hurtle 2/12, 2009 at 11:33

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. – Albaugh 2/12, 2009 at 20:35

@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... – Hunchback 3/12, 2009 at 16:20

(ah, see the F# example above for an illustration of the lambda implementation wossit - I knew I'd seen it somewhere!) – Hunchback 3/12, 2009 at 16:21

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 – Poree 7/7, 2011 at 9:19

M

149

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")