Magic strings in ASP.NET MVC

Question

I have a background in desktop software development and am getting started with learning ASP.NET MVC.

In my default HomeController I have the Index action which has code that looks like this:

if (!Request.IsAuthenticated)
    return RedirectToAction("Login", "Account");

In other words, redirect the user to "/account/login". The AccountController.Login action will then handle the user and send him back to the HomeController once he logs in successfully.

This code smells to me, perhaps just because I'm accustomed to doing things differently in desktop software. What if I change the name of the Login action to "LogOn"? What if I remove the AccountController altogether and replace it with something else? I will introduce a new bug but I won't get compiler errors, and my unit tests probably won't catch it either. Since I used strings to specify controller and action names, refactoring and redesigning has more potential to break code all over the place.

What I would like is something like this:

if (!Request.IsAuthenticated)
    return RedirectToAction(() => AccountController.Login);

However I'm not sure if that's even possible or if it's the best way to do it.

Am I being stupid, or have other people had the same problem? What do you do to get around it?

Nasalize

Answer 1

I think what you're looking for is the reason why T4MVC exists - it removes all of the "magic strings" relating to controllers and actions and replaces them with classes and properties.

With T4MVC, this

if (!Request.IsAuthenticated)
    return RedirectToAction("Login", "Account");

becomes this

if (!Request.IsAuthenticated)
    return RedirectToAction(MVC.Account.Login());

there is a flag that can be set in the T4MVC settings to force it to run the template on each build, giving you early warning when something might have changed.

Although not what you asked, you may consider using the AuthorizeAttribute to remove the need to check if the request is authenticated inside of your controller action.

this

public class HomeController : Controller
{
    public ActionResult Index() 
    {
        if (!Request.IsAuthenticated)
            return RedirectToAction("Login", "Account"); 

        // .... carry on
    }
}

becomes

public class HomeController : Controller
{
    [Authorize]
    public ActionResult Index() 
    {
        // .... carry on
    }
}

then in web.config, set the url to point at the account login URL

<authentication mode="Forms">
   <forms loginUrl="account/login" timeout="30" />
</authentication> 

Granted, this doesn't give you any safety if your controllers and actions change (similar to your original complaint), but you can always set up a route to direct the chosen URL to the right controller and action and use the T4MVC generated classes in the route, providing you with some compile time warning if things have changed.

Historiographer

Wow that's so useful. I wish / hope they integrate this directly into MVC in the future. I don't see how using so many magic strings can be acceptable these days. – Crisis

Answer 2

In C# 6 you can take advantage of nameof and easily refactor many of these magic strings.

... = new SelectList(context.Set<User>(), nameof(User.UserId), nameof(User.UserName));

...
return RedirectToAction(nameof(Index));

Cassicassia

Answer 3

You can write your own custom extension methods to help you avoid magic strings. For example you can see my implementation here: https://github.com/ivaylokenov/ASP.NET-MVC-Lambda-Expression-Helpers Keep in mind this adds a little bit of performance overhead run-time which you can solve by caching all the links. If you want compile time checking T4MVC is your solution: http://t4mvc.codeplex.com/

Reasoned

Answer 4

I know it's not really an answer, but using a tool like Resharper can help you solve this too. Resharper keeps track of controllers and actions and issues a warning if some magic string that's supposed to be a Controller isn't one. It only works on standard methods like RedirectToAction or ActionLink though.

Edit: Apparently you can add annotations to make it work with your custom extension methods. See here.

Grasmere

Answer 5

If you're really only concerned about redirecting to actions and controllers, the C# 6 nameof operator is really helpful - until you need to redirect between controllers. I ended up writing a utility method to get the controller name:

public class Utilities
{
    public static string ControllerName<T>() where T : Controller
    {
        var name = typeof(T).Name;
        return name.Substring(0, Math.Max(name.LastIndexOf(nameof(Controller), 
                                          StringComparison.CurrentCultureIgnoreCase), 0));
    }
}

And you can use it like this:

public ActionResult Foo()
{
    return RedirectToAction(nameof(HomeController.Index), 
                            Utilities.ControllerName<HomeController>());
}

Nita

Answer 6

Russ's answer is right on the money, however it doesn't really address your concern.. which is really "Why are there magic strings in MVC?".

In early versions of MVC, it was actually quite different. They didn't use the magic strings and had more type based methods. However, this was changed for a number of reasons. I've forgotten the details, but there were logical reasons for this otherwise strange reversion to non-typesafe methods.

I seem to recall it might have had something to do with the way MVC searches multiple areas for matching conventions, and this was much easier to achieve with strings than typed objects. This is particularly true of Views and partials.

Maybe someone that remembers the details can chime in.

Shelf

Answer 7

As an alternative to T4MVC, I have written a small helper that allows you to use pretty much exactly your suggested syntax and without the need for automatically generated code:

<a href="@(Url.To<MyController>().MyAction("foo", 42))">link</a>

And in the controller:

return Redirect(Url.To<MyController>().MyAction("foo", 42).ToString());

My solution works by doing at runtime what T4MVC does at build time.

There is a NuGet package and a GitHub project.

Halvah