Combating magic strings in a web application

I'm on a quest to combat all the magic strings which tends to pile up in an ASP.NET MVC application.

All reviews will be highly appreciate but please note that:

• Some of the classes contains more (unrelated) code than I've presented here.

• I do not use the c/c++ type aliases (unless forced to as with base type of an enum).

• I use the this keyword for scoop readability. (Exception: razor syntax)

Examples

1) Getting an action URL in a view. (before/after)

Simple .NET Fiddle

Similar users


((c) => c.Match(Model.Name, Model.Age)))">Similar users


2) Performing an action redirect in controller. (before/after)

return this.RedirectToAction("Login", "Account", new { redirectUrl = url });


return this.RedirectToAction((c) => c.Login(url));


Code

MvcExtensions.cs

public static class MvcExtensions
{

public static String Action(this UrlHelper urlHelper, Expression> expression) where TController : Controller
{

if (urlHelper == null)
{
throw new ArgumentNullException(nameof(urlHelper));
}
else if (expression == null)
{
throw new ArgumentNullException(nameof(expression));
}

var info = ActionInfo.Create(expression);

if (info.RouteValues.Count == 0)
{
return urlHelper.Action(info.ActionName, info.ControllerName);
}

return urlHelper.Action(info.ActionName, info.ControllerName, info.RouteValues);

}

}


MvcController.cs

```
public class MvcController : Controller
{

public MvcController()
{
}

protected RedirectToRouteResult RedirectToAction(Expression> expression) where TController : Controller
{

if (expression == null)
{
throw new ArgumentNullException(nameof(expression));
}

var info = ActionInfo.Create(expression);

if (info.RouteValues.Count == 0)
{
return base.RedirectToAction(info.ActionName, info.ControllerName);

First point, these extensions are really cool.

I don't have much comments to give, your code looks pretty good!

The ActionInfo class could be simplified a little :

internal sealed class ActionInfo
{
    private ActionInfo(string actionName, string controllerName, RouteValueDictionary routeValues)
    {
        ActionName = actionName;
        ControllerName = controllerName;
        RouteValues = routeValues;
    }

    public String ActionName { get; }
    public String ControllerName { get; }
    public RouteValueDictionary RouteValues { get; }

    internal static ActionInfo Create(Expression> expression) where TController : Controller
    {
        if (expression == null)
        {
            throw new ArgumentNullException(nameof(expression));
        }

        var body = (MethodCallExpression)expression.Body;
        var routeValues = new RouteValueDictionary(Utils.GetMethodParameters(body));
        String actionName = Utils.GetActionNameFromMethod(body.Method);
        String controllerName = Utils.GetControllerNameFromType(typeof(TController));

        return new ActionInfo(actionName, controllerName, routeValues);
    }
}

Overall, you didn't need to members, you can use read-only properties from C#6.

You have a tendency for useless parenthesis! For example here :

if ((attribute != null) && !String.IsNullOrEmpty(attribute.Name))

(attribute != null) doesn't need parenthesis. They bulk the code and it serves no purpose.

Finally, this doesn't need the else, the code flow will never take advantage of the else if, a simple if would do the same work.:

if (urlHelper == null)
{
    throw new ArgumentNullException(nameof(urlHelper));
}
else if (expression == null)
{
    throw new ArgumentNullException(nameof(expression));
}

StackExchange Code Review Q#110650, answer score: 7