Question

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!

Was it helpful?

Solution

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.

OTHER TIPS

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.

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.

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

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top