Sending email for allotment

Question

Requirement:

If the bookshop manager field changes from A to B, I need to email the person B saying that the new Bookshop has been allotted to him and he has to do certain activities.

private void SendEmailToAllottedManager()
{
   EmailExpress mEmailExpress = new EmailExpress();
   mEmailExpress.SendEmail(Shop.Bookstore.BookstoreName, Subject);
}

In the method where I am calling SendEmailToAllottedManager, I am checking if the Manager has indeed changed or not as below:

if (OriginalManager != CurrentManager)
{
    SendEmailToAllottedManager();
}

During code review, a colleague suggested that I move the check from outside the function SendEmailToAllottedManager() to inside it:

Proposed Code

private void SendEmailToAllottedManager()
{
    if (OriginalManager != CurrentManager)
    {
          EmailExpress mEmailExpress = new EmailExpress();
          mEmailExpress.SendEmail(Shop.Bookstore.BookstoreName, Subject);
    }

}

I maintain my stand that the function SendEmailToAllottedManager is only about emailing and not about performing the checks, etc.

What do you think?

Answer 1

I would say, that actually sending the email is part of the process of changing the store manager. It's part of the business logic (AFAIK), that when a store changes it's manager, the new manager should be notified, among other things.

Thats why the check if the manager changes is not a job for the email sender at all. The email sender should be a utility, which can be reused.

Answer 2

I agree with you. The performed action of a method should correspond to it's name. In your colleague's suggestion, an email sent to the allotted manager is, as you already pointed out, not always being sent and thus not 100% semantically accurate.

However, for the sake of readability, it sometimes can be easier to put the if-then check in the same method, especially if you have a lot of if-then checks to be performed. But then you might want to consider doing some refactoring instead, if it becomes too bloated. It's always hard to know where to draw the line.

Interesting question! Im looking forward to seeing others thoughts.