How to keep controllers small in ASP.NET MVC?

Question

I have a pretty well designed architecture where controllers go to services which access repositories which communicate with the database.

As such, logic in controllers is kept at a minimum, but I still have very subtle pieces of code that perform some tasks like

validate the model
arrange the action method arguments
invoke some service with these arguments, maybe validate the result and return the view if the model is now invalid
finally produce a model from the service's result, and return that.

some longer cases do different things depending on a "status" returned by the service.

here's a couple of examples:

[HttpPost]
[AjaxOnly]
[Authorize]
public JsonResult Preview(string input)
{
    LinkResult parsed = linkService.ParseUserInput(input);
    if (parsed.Result == LinkParseResult.Used)
    {
        long? postId = parsed.Link.PostId;
        if (postId.HasValue)
        {
            Post post = postService.GetById(postId.Value, false);
            return Json(new
            {
                faulted = "used",
                link = DetailsRoute(post),
                id = postId
            });
        }
        else
        {
            return Json(new { faulted = "invalid" });
        }
    }
    else if (parsed.Result == LinkParseResult.Invalid)
    {
        return Json(new { faulted = "invalid" });
    }
    else
    {
        Link link = parsed.Link;
        if (link.Description != null && link.Description.Length > 200)
        {
            link.Description = link.Description.Substring(0, 200);
        }
        return AjaxView(link);
    }
}

and (Post comes from domain, PostModel is the view model)

private PostModel PostModelConverter(Post post)
{
    Link link = post.Link;
    if (link == null)
    {
        throw new ArgumentException("post.Link can't be null");
    }
    if (link.Type == LinkType.Html)
    {
        return new PostedLinkModel
        {
            Description = link.Description,
            PictureUrl = link.Picture,
            PostId = post.Id,
            PostSlug = postService.GetTitleSlug(post),
            Timestamp = post.Created,
            Title = link.Title,
            UserMessage = post.UserMessage,
            UserDisplayName = post.User.DisplayName
        };
    }
    else if (link.Type == LinkType.Image)
    {
        return new PostedImageModel
        {
            PictureUrl = link.Picture,
            PostId = post.Id,
            PostSlug = postService.GetTitleSlug(post),
            Timestamp = post.Created,
            UserMessage = post.UserMessage,
            UserDisplayName = post.User.DisplayName
        };
    }
    return null;
}

this raises the question about if view models really should be in the web project as a rule, or they could actual be part of the domain, or some other project.

I'm not sure I can do much about the preview action, other than maybe use a PreviewModel that receives the Link, and truncates the description, but that would save up like, two lines.

The model converter should probably be somewhere else, but I'm clueless as to where that should be.

Another point that comes to mind is if I should be splitting this controller either using the partial keyword (is it a bad practice to use this for something else than autogenerated classes?), or adding routes that use different controllers depending on what action is requested or what http method is being used, what's the usual way to handle that?

Just a personal preference, but you can use switch statements instead of if .. else if ... etc. — Master Morality, Jul 31 '12 at 20:50

score 5 · Accepted Answer · edited May 23 '17 at 12:21

This has been asked several times:
Business logic in the controller
Where should I put my controller business logic in MVC3
Keep Controllers Thin

As well as written about elsewhere:
ASP MVC Best Practices - Skinny Controllers
Keep Controllers Thin

The community seems to be at a good consensus that this kind of logic belongs outside the controllers. Generally in the model (or ViewModel), but somewhere in the business layer.

As a final note, using partials for non-autogenerated code is not discouraged. If it makes sense to split things, do so. Just think about about what your reasons for splitting it are. This is going to be a case-by-case kind of thing.

score 2 · Answer 2 · answered Jul 31 '12 at 20:42

private PostModel PostModelConverter(Post post)
{
    Link link = post.Link;
    if (link == null)
    {
        throw new ArgumentException("post.Link can't be null");
    }
    if (link.Type == LinkType.Html)
    {
        var model = AutoMapper.Map<PostedLinkModel>(post);
        model.PostSlug = postService.GetTitleSlug(post);
        return model;
    }
    else if (link.Type == LinkType.Image)
    {
        var model = AutoMapper.Map<PostedImageModel>(post);
        model.PostSlug = postService.GetTitleSlug(post);
        return model;
    }
    return null;
}

http://www.viddler.com/v/b568679c

score 0 · Answer 3 · edited Jun 20 '20 at 09:12

Controller will not contain any Domain logic

Controller should be only responsible for:

Validating Input

Calling Model to prepare the view

Return the view or redirect to another action

If you are doing any other thing you are doing it in a wrong place, it is rather the Model responsibility which you are doing in Controller.

If you follow this rule your action method will not be more than 20 – 25 lines of code. Ian Cooper has an excellent post Skinny Controller Fat Model, do read it.

How to keep controllers small in ASP.NET MVC?

3 Answers3