views:

156

answers:

4

I'm looking for recommendations on how to approach the following design problem (using a fictitious example based on stackoverflow). I'd trying to avoid an anemic domain model and seek general "best-practice" advice for this type of case.

Scenario:

Suppose a new feature is being developed for stackoverflow that sends an email notification to a question's owner whenever his/her question receives 10 upvotes.

The domain object model is something like this:

public class Question
{
    string Question { get; set; }
    IList<Votes> Upvotes { get; set; }
    User Owner { get; set; }

    public void AddUpvote(Vote upvote)
    {
        Upvotes.Add(upvote);
    }
}

Potential Implementations:

  1. Change AddUpvote() to take an IEmailerService parameter and perform the logic within the AddUpvote() method.

    public void AddUpvote(Vote upvote, IEmailerService emailer)
    {
        Upvotes.Add(upvote);
        if ( Upvotes.Count == 10 )
        {
            emailer.Send(Owner.EmailAddr);
        }
    }
    
  2. Detect this state within AddUpvote() and have AddUpvote() resolve an IEmailService from an IoC container (instead of passing the IEmailerService as a parameter).

  3. Detect this state in the external service object that invokes question.AddUpvote().

    public void UpvoteClickHandler(Question question)
    {
        question.AddUpvote(new Upvote());
        if ( question.Upvotes.Count == 10 )
        {
            _emailer.Send(question.Owner.EmailAddr);
        }
    }
    
  4. Your better solution here!

+2  A: 

Both options 1) and 2) jump out as being the wrong place to send out an email. A Question instance shouldn't know these two things:

  1. It shouldn't know about the policy, ie when to send out an email.
  2. It shouldn't know about the mechanics of notification for a policy, ie the email service.

I know that this is a matter of taste, but you're tying in the Question closely with both a policy as well as the mechanism to send out an email. It would be really hard to move this Question class to another project (like ServerFault, which is StackOverflow's sister site for instance)

I'm interested in this question, because I'm creating a notification system for a Help Desk that I am building. This is what I did in my system:

Create a NotificationManager (Basically, completely move the concern of notifications to a separate class).

public Class NotificationManager
{
    public void NotificationManager(NotificationPolicy policy, IEmailService emailer)
    {
    }
}

I then did something along the lines of this (The UpvoteClickHandler has a dependency to a NotificationManager instance):

public void UpvoteClickHandler(Question question)
{
    question.AddUpvote(new Upvote());
    _notificationManager.Notify(Trigger.UpvoteAdded, question);
}

All the UpvoteClickHandler does is tell NotificationManager that an upvote was added to question and let NotificationManager determine whether and how it should send out an email.

Praveen Angyan
+1  A: 

The answer depends on your fundamental approach to application and object design. And (edit here) what you view as your most important trait of the system. Looks like you have data, questions, and business rules, up votes. Not question objects at all. So you should treat your data as data and allow data tools to work on them, by not mixing behavior into them. Traditional object design would have all the behaviors and data in the object, so sending eMail would belong in the object. (option 1 and 2) I guess this is the black box, or self contained object approach. Modern practices, as I've come to learn, has objects as simple data holders. Which are meant to be moved around, persisted, transformed and have stuff done to them. Perhaps living as little more than structs in C. The behavior comes from the services and transformations that are applied to the simple objects.

Martlark
What you have described is known as the Anemic Domain Model, arguably the antithesis of OO and something the poster is specifically trying to avoid. Data + behavior = object.
Bryan Watts
Well indeed he did say he wanted to avoid it, but then introduced an option, 3, which brought it back.
Martlark
I wouldn't say that "modern" design involves nothing but a bunch of DTO's and services. SOA is a philosophy that has been around since the early days of OO, so to say this approach is somehow more modern is erroneous. SOA, and OO go hand in hand. It is true, that a the "heavy" domain model is falling out of favor, but only because so much bad OO existed already. Services are meant to deal with a contract, and thus business logic must be stripped out before passing that information around. To do this you need DTO's, but a design that uses only DTO's is pretty much useless.
Josh
+5  A: 

You really don't want to mix these two together since they have separate concerns. Let the Question class care about questions and the message service care about what to do when the voting hits 10, or 20, or 100 or...

The following example is meant for demonstration purposes only, but you will get the point. There is a clear separation of concerns, so the Question class doesn't have to change if the requirements for sending messages changes. Remember according to the SOLID principles, a class should only have one reason to change.

public class Question
{
    public string Description { get; set; }
    public Int32 Votes { get; set; }
    public User Owner { get; set; }

    public event EventHandler<QuestionEventArgs> OnUpvote;

    private void RaiseUpvoteEvent(QuestionEventArgs e)
    {
     var handler = OnUpvote;
     if (handler != null) handler(this, e);
    }

    public void Upvote()
    {
     Votes += 1;

     RaiseUpvoteEvent(new QuestionEventArgs(this));
    }
}

public class MessageService
{
    private Question _question;

    public MessageService(Question q)
    {
     _question = q;

     q.OnUpvote += (OnUpvote);
    }

    private void OnUpvote(object sender, QuestionEventArgs e)
    {
     if(e.Question.Votes > 10)
      SendMessage(e.Question.Owner);
    }
}

public class QuestionEventArgs: EventArgs
{
    public Question Question { get; set; }

    public QuestionEventArgs(Question q)
    {
     Question = q;
    }
}

So there you have it. There are a lot of other ways to accomplish this, but the event model is a great way to go, and it accomplishes the separation of concerns you want in your implementation in order to make maintenance earlier.

Josh
It seems odd that you're saving a reference to the question in the MessageService constructor yet acting upon whichever instances is passed into OnUpvote.
Todd Smith
Yes, in practice I would never use a reference type like this in an event unless absolutely necessary. However, this is only meant as a sample of how to separate the functionality from the Question object. If I were implementing a real service here, the same handling method would probably handle a collection of services, and the event would probably pass around a question ID or something.
Josh
+1  A: 

HI all,

In my opinion "sends an email notification to a question's owner whenever his/her question receives 10 upvotes" is domain logic and therfore it should be into domain object, in order to avoid an anemic domain.

It's the action of sending the email (i.e. communicate with smtp server) that MUST go into the infrastructure layer.

So i think that option 1 is not totally wrong. Keep in mind that you can always test your object by passing a mock implementation of the IEmailerService.

Best Regards,

Stefano