Should all business logic be in domain models or?

Question

ASP.NET MVC4 - Basically I used to have all my business logic in my controllers (which I'm trying to put into the domain models instead). However I don't quite know if ALL my business logic should be put into the domain models or if some should remain in the controllers?

For instance I got a controller action as shown below:

[HttpPost]
    public ActionResult Payout(PayoutViewModel model)
    {
        if (ModelState.IsValid)
        {
            UserProfile user = PublicUtility.GetAccount(User.Identity.Name);
            if (model.WithdrawAmount <= user.Balance)
            {
                user.Balance -= model.WithdrawAmount;
                db.Entry(user).State = EntityState.Modified;
                db.SaveChanges();

                ViewBag.Message = "Successfully withdrew " + model.WithdrawAmount;
                model.Balance = user.Balance;
                model.WithdrawAmount = 0;
                return View(model);
            }
            else
            {
                ViewBag.Message = "Not enough funds on your account";
                return View(model);
            }
        }
        else
        {
            return View(model);
        }
    }

Now should all the logic be put into a method in a domain model so the action method looks like this?

[HttpPost]
    public ActionResult Payout(PayoutViewModel model)
    {
        var model = GetModel(model);
        return View(model);
    }

Or how would you go around doing it?

Answer 1

We put our application and business logic into separate layers (csproj file) a domain layer for business logic and a service layer for application logic. This abstracts them out from the MVC project completely. This has two big benefits for us. The first is that the business logic isn't tied to a pattern that could change. A few years ago none of us would have imagined the popularity of MVC today, and in a a few years we don't know if there will be some new thing that will come along and replace MVC so getting the vast majority of your code to be "un-tied" to MVC would help should you ever want to abandon MVC for something else.

The second benefit is it makes having different presentation layers very easy to implement. So if you wanted to present your business logic as a WCF service you could do that very easily by creating a new WCF project and making that a façade for your Service and domain layers. It makes maintenance very easy since both your MVC project and your WCF service would be using the same Business Logic classes.

Example Below is some example code of what I would do. This is quick and dirty and there should be more to it like adding logging if the user doesn't save back to the database etc...

public enum PayoutResult
{
    UserNotFound,
    Success,
    FundsUnavailable,
    DBError
}

public class UserProfile
{
    public float Balance { get; set; }

    public string Username { get; set; }

    // other properties and domain logic you may have

    public bool Withdraw(PayoutModel model)
    {
        if (this.Balance >= model.Amount)
        {
            this.Balance -= model.Amount;
            return true;
        }

        return false;
    }
}


public class PayoutService
{
    IUserRepository userRepository;

    public PayoutService()
    {
        this.userRepository = new UserRepository();
    }

    public PayoutResult Payout(string userName, PayoutModel model)
    {
        var user = this.userRepository.GetAll().SingleOrDefault(u => u.Username == userName);
        if (user == null)
        {
            return PayoutResult.UserNotFound;
        }

        // don't set the model properties until we're ok on the db
        bool hasWithdrawn = user.Withdraw(model);
        if (hasWithdrawn && this.userRepository.SaveUser(user))
        {
            model.Balance = user.Balance;
            model.Amount = 0;

            return PayoutResult.Success;
        }
        else if (hasWithdrawn)
        {
            return PayoutResult.DBError;
        }

        return PayoutResult.FundsUnavailable;
    }
}

Your controller would now look like this

[HttpPost]
public ActionResult Payout(PayoutModel model)
{
    if (ModelState.IsValid)
    {
        var result = service.Payout(User.Identity.Name, model);
        // This part should only be in the MVC project since it deals with 
        // how things should be presented to the user
        switch (result)
        {
            case PayoutResult.UserNotFound:
                ViewBag.Message = "User not found";
                break;
            case PayoutResult.Success:
                ViewBag.Message = string.Format("Successfully withdraw {0:c}", model.Balance);
                break;
            case PayoutResult.FundsUnavailable:
                ViewBag.Message = "Insufficient funds";
                break;
            default:
                break;
        }               
    }

    return View(model);
}

And if you had to expose the payout in a web service (I work in an enterprise environment so this happens a lot for me) You do the following...

public class MyWCFService : IMyWCFService
{
    private PayoutService service = new PayoutService();

    public PayoutResult Payout(string username, PayoutModel model)
    {
        return this.service.Payout(username, model);
    }
}

Answer 2

For me, the separation of concerns is the most important guiding principle for these decisions. So, it depends on how complex your domain is and what benefit you get from complicating the code.

Anyway, as a general rule, I tend to give Controllers the following concerns:

1. Instantiation and mapping of view models (unless there is considerable mapping)
2. View model validation

And, I tend to refer to a model (or service) for non-application specific domain knowledge:

1. Can withdraw money
2. Make withdrawal

So, this is how I would split the code:

    [HttpPost]
    public ActionResult Payout(PayoutViewModel model)
    {
        if (ModelState.IsValid)
        {
            var account = accountRepository.FindAccountFor(User.Identity.Name);

            if (account.CanWithdrawMoney(model.WithdrawAmount))
            {
                account.MakeWithdrawal(model.WithdrawAmount);

                ViewBag.Message = "Successfully withdrew " + model.WithdrawAmount;
                model.Balance = account.Balance;
                model.WithdrawAmount = 0;
                return View(model);
            }

            ViewBag.Message = "Not enough funds on your account";
            return View(model); 
        }
        else
        {
            return View(model);
        }
    }

The saving of the application state, I usually wrap up in an interceptor. That way you can wrap a unit of work transaction around the entire request.

Answer 3

I would put all the logic in the domain model, and do two calls to the domain, one for validation, one for executing the use case.

So the entity looks like this:

public class User 
{
    public double Balance { get;set; }

    public ValidationMessageCollection ValidatePayout(double withdrawAmount)
    {
        var messages = new ValidationMessageCollection();

        if (withdrawAmount > Balance)
        {
            messages.AddError("Not enough funds on your account");
        }

        return messages;
     }

     public void Payout(double withdrawAmount)
     {
         balance -= withdrawAmount;
     }
 }

And your controller would look like this:

[HttpPost]
public ActionResult Payout(PayoutViewModel model)
{
    if (!ModelState.IsValid)
    {
        return View(model);
    }

    var user = PublicUtility.GetAccount(User.Identity.Name);
    var validationMessages = user.ValidatePayout(model.WithdrawAmount)

    if(validationMessages.Any())
    {
        ViewBag.Message = validationMessages.ToSummary();
        return View(model);
    }

    ViewBag.Message = "Successfully withdrew " + model.WithdrawAmount;
    model.Balance = user.Balance;
    model.WithdrawAmount = 0;
    return View(model);
}

There are other things I would do, like insert an application/service layer, use viewModels and do all the resetting of the ViewModel in a ViewModelBuilder/Mapper or simmilar, but this shows the basic idea.

Answer 4

The approach which we follow has required business cases enclosed within ViewModel (Your case: PayoutViewModel) and exposed through method and these method will be consumed within controller actions. In addition, we have a clear seperation on model and view model where viewmodel refer model within it.

Answer 5

It is recommended that you have a thin code on controllers , it's better you handle business logic in another layers like serviceLayer that i have used it before which the returns you a view model of what you have wanted to return to your view/controller. Even define your ajax methods inside a service layer class.Which decrease code complexity and maintainability problems.Even it is more readable ..

In you Controller you can use DI to inject the serviceLayer class or instansiate it as

  ServiceLayer test = new ServiceLayer() ; 

then in you controller

  test.registerCustomer(model); // or
  test.registerCutomer(CustomerViewModel);