Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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 Linq2SQL

This means that currently the controller for the action /Account/Add will perform the following pseudo-code:

  • Get the current user
  • new() an AccountItem object
  • Set the UserId, AccountItemTypeId and Value fields on the AccountItem
  • Get an instance of Repository
  • Call the repositorys InsertOnSubmit(AccountItem item) method
  • Call the repositorys SubmitChanges() method

  • new() an EmailQueue item

  • Set the appropriate properties on the EmailQueue item (to, subject, body, etc,..) based on the inserted account item
  • Get an instance of Repository
  • Call the repositorys InsertOnSubmit(EmailQueue item) method
  • Call the repositorys SubmitChanges() method

I'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 method

Should 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?

share|improve this question
    
I think my question is a good fit for programmers.se, if it's not any help in refactoring it (please excuse the somewhat unintentional pun!) to fit would be appreciated =) –  Rob Feb 6 '12 at 19:21
add comment

1 Answer

If the business rule is that AddAccountItem must AddEmailToQueue every time it is successful then I would use the dependency. Otherwise you will have to make sure you wire it up every time.

I feel that email rules are business logic not presentation logic and would keep it out of the controller.

share|improve this answer
    
I like your answer but according to DDD I'd separate between Domain logic and Application logic. The latter can live in a controller and it's perfectly fine to be called and orchestrated there. –  Falcon Feb 6 '12 at 19:40
    
It's a tough call between business rules and DDD. It really depends on the individual instance and how often it will be called as well as the design of the rest of the system. –  Amy Patterson Feb 6 '12 at 21:17
    
The decision as to whether an email is sent or not is made by checking the value of a record retrieved from a Repository<ConfigurationItem>; well several actually as there's a system default and user specific overrides. I'm now leaning towards injecting an IEmailService into the IAccountService (with IEmailService taking an ICondigurationService to determine if the email needs to be sent or not) –  Rob Feb 7 '12 at 6:20
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.