Fat controllers : How do i make it slim?

Question

I am developing a blog engine using EF 6 and MVC 5.

I have decided not to use Repository pattern or UoW as it is already implemented in EF 6 at framework level.

The solution contains following layers.

DataModels layer: It has simple POCO's which are auto generated & a dbContext.

public partial class Article
    {
        public int Id { get; set; }
        public string Slug { get; set; }
        public string Title { get; set; }
        public string PostBody { get; set; }
        public System.DateTime CreatedOn { get; set; }
        public bool IsPublished { get; set; }
        public string Author { get; set; }
    }

Service Layer:

public interface IBlogEngine
    {
        List<Article> GetFrontPageBlogPosts();
        void SaveArticle(Article article);
        List<Article> GetArticlesByStatus(string isPublished);
        Article GetBySlug(string slug);
        Article GetById(int id);
        bool Exists(string slugUrl);
        void Delete(int id);
    }

IBlogEngine implementation. Some methods implementation are omitted for brevity.

public class BlogEngine : IBlogEngine
    {
        private readonly dbContext _context;

        public BlogEngine(DbContext context)
        {
            _context = context;
        }


        public void SaveArticle(Article article)
        {
            if (article.Id == 0)
            {
                _context.Articles.Add(article);
            }
            else
            {
                _context.Entry(article).State = EntityState.Modified;
            }

            _context.SaveChanges();
        }



        public Article GetBySlug(string slug)
        {
            return _context.Articles.SingleOrDefault(x => x.Slug == slug.Trim());
        }


    }

UI Layer

 public class ArticleController : Controller
    {
        private readonly IBlogEngine _engine;
        public ArticleController(IBlogEngine engine)
        {
            _engine = engine;
        }

        [HttpGet]
        public ActionResult Edit(string slug)
        {
            if (string.IsNullOrWhiteSpace(slug))
            {
                return HttpNotFound();
            }

            var article = _engine.GetBySlug(slug);

            if (article == null)
            {
                return HttpNotFound();
            }

            var model = new EditViewModel { Id = article.Id, Slug = article.Slug, 
            Title = article.Title, PostBody = article.PostBody, IsPublished = true };

            return View("Create", model);
        }

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Edit(EditViewModel blogPost)
        {
            if (!ModelState.IsValid)
            {
                return View("Create", blogPost);
            }
            // Get Article by Id
            var article = _engine.GetById(blogPost.Id);

            if (article == null)
            {
                return HttpNotFound();
            }

            // Update it
            article.Id = blogPost.Id;
            article.Title = blogPost.Title.Trim();
            article.Slug = blogPost.Slug.ToUrlSlug();
            article.PostBody = blogPost.PostBody;
            article.CreatedOn = DateTime.UtcNow;
            article.IsPublished = blogPost.IsPublished;
            article.Author = User.Identity.Name;

            // Save it
            _engine.SaveArticle(article);

            return RedirectToAction("Create", "Article");
        }

    }

The Problem Consider a scenario where a user is finished editing his old blog post/article and hits submit button to update he blog post/article.

Is my HTTP POST Edit action too fat ? I feel that the controller is doing too may things here.

1. Get the existing article from DB

2. Update it with ViewModel values

3. Call SaveArticle method from service layer.

How can i put this controller on diet ?

Shouldn't the Service Layer method SaveArticle do the job of retrieving an article from Db and update it with new values and call SaveChanges method ?

If above statement is true, How can i pass the ViewModel to ServiceLayer method ? Isn't it a bad decision to allow ViewModels leak into Service layer ?

How do i handle this ? I am confused and need some help.

Answer 1

Frankly, it gets confusing to me as well sometimes as to what and how much the controller should be doing to serve a request.

In most of my implementation I do following:

1. Accept the viewModel input object in Post method (the input values are validated at client side).
2. Check the ModelState.
3. Convert the viewmodel object into domain model object. I use AutoMapper.
4. Give it to service method. It does what needs to be done for the operation.
5. Return appropriate result based on operation.

I were you, I would write:

    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult Edit(EditViewModel blogPost)
    {
        if (!ModelState.IsValid)
        {
            return View("Create", blogPost);
        }

        // Use AutoMapper for ViewModel to DomainModel conversion
        var blogPostDomainModel = Mapper.Map<EditViewModel, BlogPost>(blogPost);

        // Save it - Update the object in persistent store. It may throw
        // exception if something wrong while updating the object. Having
        // validated input from UI that should only happen due to server
        // error.
        _engine.SaveArticle(blogPostDomainModel);

        return RedirectToAction("List", "Article");
    }