Product I've inherited has the following composition:MVC web site where the controllers make calls to Repositories to get, insert and update objects retrieved that are defined in the:Data access layer written using Linq2SQLThis means that currently the controller for the action /Account/Add will perform the following pseudo-code:Get the current usernew() an AccountItem objectSet the UserId, AccountItemTypeId and Value fields on the AccountItemGet an instance of RepositoryCall the repositorys InsertOnSubmit(AccountItem item) methodCall the repositorys SubmitChanges() methodnew() an EmailQueue itemSet the appropriate properties on the EmailQueue item (to, subject, body, etc,..) based on the inserted account itemGet an instance of RepositoryCall the repositorys InsertOnSubmit(EmailQueue item) methodCall the repositorys SubmitChanges() methodI've already made a first pass through the codebase and injected, using Castle Windsor, the repository's in (actually injecting an instance of a type that implements IRepositoryResolver which has a method called GetRepository<T> so that I can decouple the controllers from the Linq2SQL dependent repositories. I've also started to write unit tests, based on the current behaviour of the controllers (to ensure that when I make further changes I can identify when I get it horribly wrong) but that's tangenital to my actual question.I've identified lots of places where the controller actions are performing identical actions against various repositories, such as getting a given users total Value for all AccountItem's and am now considering extracting all this common logic into a business/service layer. My question is, would the following structure be considered best practice:AccountService: Provides an AddAccountItem method (plus others such as GetAccountValue)EmailService: Provides an AddEmailToQueue methodShould I be refactoring the controller so it does:accountServiceInstance.AddAccountItem(userToCredit, userCrediting, AccountItemType.Bonus, 300);emailServiceInstance.AddEmailToQueue(userToEmail, EmailType.AccountCreditBonus);Or, should my AccountService take an EmailService as a dependency (that's satisfied by Castle Windsor) and it's the responsibility of the AccountService to call EmailService.AddEmailToQueue?
Refactoring an existing MVC codebase to remove business logic & data access from the Controllers
design;refactoring;asp.net mvc