Why did they use this C# syntax to create a list of links in ASP.NET MVC 2?

Question

I'm having hard time understanding the following C# code. This code was taken from Pro ASP.NET MVC 2 Framework by Steven Sanderson. The code esentially creates URLs based on a list of categories.

Here's the code:

        Func<string, NavLink> makeLink = categoryName => new NavLink { 
            Text = categoryName ?? "Home", 
            RouteValues = new RouteValueDictionary(new { 
                controller = "Products", 
                action = "List", 
                category = categoryName, 
                page = 1 
            }),
            IsSelected = (categoryName == currentCategory)

A lot of stuff is going on here. I'm guessing it's defining a function that expects two parameters of type string, and NavLink. Then I see the Lambda categoryName => new NavLink etc.... I think all it's doing is creating an instance of NavLink.

The function is then called in the same Controller action:

        // Place home link on top
        List<NavLink> navLinks = new List<NavLink>();
        navLinks.Add(makeLink(null));

        // Add link for each distinct category
        var categories = productsRepository.Products.Select(x => x.Category.Name);
        foreach (string categoryName in categories.Distinct().OrderBy(x => x))
            navLinks.Add(makeLink(categoryName));

I can tell that it's making a list of NavLink. I don't understand why Steven Sanderson wrote it this way though. Couldn't he have written something like:

var categories = productsRepository.Products.Select(x => x.Category.Name);
foreach (string categoryName in categories.Distinct().OrderBy(x => x))
{
    var newlink = new Navlink{
        text = categoryName,
        RouteValues = new RouteValueDictionary(new {
           controller = "Products",
           action = "List",
           category = categoryName,
           page = 1
        }),
        IsSelected = (categoryName == currentCategory)
    }
    navLinks.Add(newlink);
}

Is there an advantage to doing it Steven's way versus my way?

Answer 1

I'm guessing it's defining a function that expects two parameters of type string, and NavLink.

It take one parameter of string, and returns a NavLink.

The only advantage I see to creaing it that way is if you create NavLinks in multiple places. Then this way you have the means of creaing one all in one place --- the typical justification for a subroutine, which is pretty much all that is.

Answer 2

It looks like that he wanted to use a local function that can be used for both adding the home link and as part of the categories loop. If he did it in-line like you did he would have to almost repeat the same code just for the single home link.

Answer 3

It seems to be more about code style. I've also taken to using the more functional programming style when I became more comfortable with delegates.

Some advantages are (1) reuse of the function for future uses and (2) loops can be shortened -- it gets rather difficult to find matching braces when they're long and nested several levels.

Answer 4

(this is just opinion, not intended to offend or provoke)

Personally, I like your way better. (or a model where you just write a plain old private function that is called from within the foreach).

It's trendy to do things with lambda, but in this case you don't gain anything for it (there is no deferred execution going on). In this case, I think the lambda just makes the code a lot less clear.

Answer 5

Overuse of any advanced and cool construct just because it's advanced and cool usually doesn't result in anything but poor readability. Unless the lambda is used in some special sophisticated tricky way elsewhere, I would use a plain old private method instead...

Edit: It's worth adding that “special sophisticated tricky ways” shall be at least commented appropriately…