Combating magic strings in a web application

Question

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

<a href="@Url.Action("Match", "Users", new { name = Model.Name, age = Model.Age })">Similar users</a>

<a href="@(Url.Action<UsersController>((c) => c.Match(Model.Name, Model.Age)))">Similar users</a>

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

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

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

Code

MvcExtensions.cs

public static class MvcExtensions
{

    public static String Action<TController>(this UrlHelper urlHelper, Expression<Func<TController, ActionResult>> 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<TController>(Expression<Func<TController, ActionResult>> 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);
        }

        return base.RedirectToAction(info.ActionName, info.ControllerName, info.RouteValues);

    }

    protected RedirectToRouteResult RedirectToActionPermanent<TController>(Expression<Func<TController, ActionResult>> expression) where TController : Controller
    {

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

        var info = ActionInfo.Create(expression);

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

        return base.RedirectToActionPermanent(info.ActionName, info.ControllerName, info.RouteValues);

    }

}

ActionInfo.cs

internal sealed class ActionInfo
{

    private String actionName;
    private String controllerName;
    private RouteValueDictionary routeValues;

    private ActionInfo()
    {
    }

    public String ActionName
    {
        get
        {
            return this.actionName;
        }
    }

    public String ControllerName
    {
        get
        {
            return this.controllerName;
        }
    }

    public RouteValueDictionary RouteValues
    {
        get
        {
            return this.routeValues;
        }
    }

    internal static ActionInfo Create<TController>(Expression<Func<TController, ActionResult>> 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 = actionName,
            controllerName = controllerName,
            routeValues = routeValues
        };

    }

}

Utils.cs

internal static class Utils
{

    internal const String ControllerNameSuffix = "Controller";

    internal static String GetActionNameFromMethod(MethodInfo method)
    {

        var attribute = method.GetCustomAttribute<ActionNameAttribute>(inherit: false);

        if ((attribute != null) && !String.IsNullOrEmpty(attribute.Name))
        {
            return attribute.Name;
        }

        return method.Name;

    }

    internal static String GetControllerNameFromType(Type type)
    {

        if ((type.Name.Length > ControllerNameSuffix.Length) && type.Name.EndsWith(ControllerNameSuffix))
        {
            return type.Name.Substring(0, (type.Name.Length - ControllerNameSuffix.Length));
        }

        return type.Name;

    }

    internal static Dictionary<String, Object> GetMethodParameters(MethodCallExpression expression)
    {

        var dictionary = new Dictionary<String, Object>();
        ParameterInfo[] parameters = expression.Method.GetParameters();

        for (Int32 index = 0; (index < parameters.Length); index++)
        {

            Expression argument = expression.Arguments[index];
            String name = parameters[index].Name;
            Object value = Expression.Lambda(Expression.Convert(argument, argument.Type)).Compile().DynamicInvoke();

            dictionary.Add(name, value);

        }

        return dictionary;

    }

}

Answer 1

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<TController>(Expression<Func<TController, ActionResult>> 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));
}