Fine, make me a blog

It's a blog, okay?
Follow What I Follow | My Book Reviews | About Me | Tweets

Oct 7

HtmlHelper.ActionLink is garbage

Abstract

In a nutshell, the logic for converting an anonymous object is tightly coupled to the logic in a RouteValueDictionary.  Break this coupling, as the support for working around C# and VB.NET’s lack of support for hash literals should in no way involve dependency on System.Web.Routing.

In addition, don’t do crazy overloads of an API — realistically, we will have to wait for VB.NET and c# to get support for named parameters and default parameters.

Discussion

Allow me to introduce to you the prize winner for some of the worst use of method overloading I’ve seen in .NET:

public static MvcHtmlString ActionLink(this HtmlHelper htmlHelper, string linkText, string actionName, object routeValues, object htmlAttributes) {
return ActionLink(htmlHelper, linkText, actionName, null /* controllerName */, new RouteValueDictionary(routeValues), new RouteValueDictionary(htmlAttributes));
}

and…

public static MvcHtmlString ActionLink(this HtmlHelper htmlHelper, string linkText, string actionName, string controllerName, RouteValueDictionary routeValues, IDictionary<string, object> htmlAttributes) {
if (String.IsNullOrEmpty(linkText)) {
throw new ArgumentException(MvcResources.Common_NullOrEmpty, "linkText");
}
return MvcHtmlString.Create(HtmlHelper.GenerateLink(htmlHelper.ViewContext.RequestContext, htmlHelper.RouteCollection, linkText, null/* routeName */, actionName, controllerName, routeValues, htmlAttributes));
}

These two methods will get called in sequence when you write the following code:

Html.ActionLink("ActionName", "ActionName", "ControllerName", new { @class="flip"});

Look carefully.

There is no method that accepts a string as its third parameter.  Instead, the third parameter here is actually a parameter called routeValues, and it is of type Object.

As a result, if this ActionLink code above is in a ViewMasterPage, then the generated HTML somehow varies by context of the view you are on.  For example, the link generated on my root page is:

<a class="flip" href="/Home/ActionName?Length=14">ActionName</a>

…and… on a non-root page, the generated link is:

<a class="flip" href="/ControllerName/ActionName?Length=14">ActionName</a>

Now, just looking at the chain of overloads for generating an ActionLink makes my head spin.  However, one thing is very clear:

If you decide you want to style an ActionLink after the fact, then be very careful, because the API is full of steel bear traps.  What can immobilize a grizzly will certainly leave you in agony for hours, debugging the problem.

I’m not even sure how it generated the get parameter ?Length=14, because

Html.ActionLink("ActionName", "ActionName", "ControllerName");

does no such non-sense.

However, the more annoying problem is that it tried invoking an action on the HomeController (my root page) when it shouldn’t’ve.

After debugging this problem, I found the root cause of the problem:

public static MvcHtmlString ActionLink(this HtmlHelper htmlHelper, string linkText, string actionName, string controllerName)
{
return ActionLink(htmlHelper, linkText, actionName, controllerName, new RouteValueDictionary(), new RouteValueDictionary());
}

For some reason, the return statement is passing in a default RouteValueDictionary for the fifth parameter, which is IDictionary<string,object> htmlAttributes.

The Length=14 GET parameter on the generated ActionLink comes from the fact the string literal “Person” has properties Chars and Length.  Chars is not translated into a GET parameter because it is one of those unusual getter properties that requires a parameter.

In addition, because the overload makes the controlerName null under the hood, the link is always relative to whatever controller served the view for the current page you are on — that’s why I was getting an IIS application server error when clicking on the link from the HomeController.

This seems both potentially malicious and definitely wrong.  To workaround this problem, I wrote a new extension method where the third parameter is a string instead of just “object”:

public static MvcHtmlString ActionLink(this HtmlHelper htmlHelper, string linkText, string actionName, string controllerName, object htmlAttributes) {
return ActionLink(htmlHelper, linkText, actionName, controllerName , new RouteValueDictionary(), new RouteValueDictionary(htmlAttributes));
}

I cannot remove the dependency on RouteValueDictionary, unfortunately, because RouteValueDictionary is used here as a wrapper to turn an “anonymous class” into a Dictionary.  By the way, the only reason for such a stupid API is because the ASP.NET team are trying to reproduce Ruby’s hash literals syntax; the constructor overload RouteValueDictionary(object) is explicitly for allowing programmers to do such mickey mouse nonsense. Removing this bad API design would be a breaking change and only make matters worse.  Instead, ASP.NET MVC team should factor out the code in this constructor into a private helper method so that the fake Ruby hash literal does not depend on System.Web.Routing.

Let me be clear: The dependency on RouteValueDictionary is likely why I saw a ?Length=6 GET parameter attached to my Uri.

Long-term, the solution will be to simply not use Microsoft’s awful HtmlHelper API.  API flaws like this can be prevented in the future by using Mono’s Gendarme utility to write design rules against such insane API design practices.  As these rules are checked at the compiled assembly level, they are language agnostic; the only dependency is CIL (Common Intermediate Language).

To recap: If I used a different overload of ActionLink, then the HtmlHelpeer would generate a different link.  Not only that, but it would mysteriously populate the link with a get parameter.

Thanks

Thanks to Steve Sanderson’s tutorial Using the ASP.NET source code to debug your app, I was able to dig into ASP.NET MVC 2 Preview 2 release.  (At first I thought maybe this was just a bug with ASP.NET MVC 2 Preview 1, but after upgrading to Preview 2 the problem was still there.)


  1. z-bo posted this