Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Magic strings in ASP.NET MVC

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?

like image 787
Phil Avatar asked Dec 03 '11 21:12

Phil


3 Answers

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.

like image 107
Russ Cam Avatar answered Oct 19 '22 13:10

Russ Cam


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));
like image 5
James Sampica Avatar answered Oct 19 '22 12:10

James Sampica


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/

like image 4
Ивайло Кенов Avatar answered Oct 19 '22 14:10

Ивайло Кенов