3
\$\begingroup\$

I would like to implement a mail notification system

Notification.cs

public abstract class Notification
{
    private IEmailSender emailSender;

    public string To { get; }

    public virtual string Subject { get; }

    public virtual string Template { get; }

    public virtual object Model { get; }

    public Notification(IEmailSender emailSender, string to, object model)
    {
        this.emailSender = emailSender;
        this.To = to;
        this.Model = model;
    }

    public virtual async Task SendAsync()
    {
        var engine = new RazorLightEngineBuilder()
            .UseFilesystemProject(Directory.GetCurrentDirectory())
              .UseMemoryCachingProvider()
              .Build();

        string message = await engine.CompileRenderAsync(this.Template, this.Model);

        await emailSender.SendEmailAsync(To, Subject, message);
    }
}

RegisterConfirmationNotification.cs

public class RegisterConfirmationNotification : Notification
{
    public RegisterConfirmationNotification(IEmailSender emailSender, string to, object model)
        : base(emailSender, to, model)
    {

    }

    public override string Subject => "Account Confirmation";

    public override string Template => "Views/EmailTemplates/RegisterConfirmation.cshtml";
}

AccountController.cs

var notification = new RegisterConfirmationNotification (emailSender, model.Email, new RegisterConfirmationViewModel { FirstName = "John" } );
await notification.SendAsync();

My problem is that I have the impression to mix a lot of things that should not be mixed. My class is responsible for compiling the Razor template but also sending the mail.

How could we separate things?

\$\endgroup\$
  • \$\begingroup\$ Is this an ordinary program or maybe an asp.net-core web-service? \$\endgroup\$ – t3chb0t Apr 8 '18 at 9:05
  • \$\begingroup\$ asp.net-core mvc with asp.net-core Identity 2.0 \$\endgroup\$ – user149517 Apr 8 '18 at 9:07
  • 1
    \$\begingroup\$ In case you are not aware of that... you don't need any external packages for that. All you need is to implement a custom middleware that intercepts the view returned as a response that you can send via email. Maybe my question that solves the same thing helps you Middleware for sending PartialViews via email. Your controller would just return a PartialView instead of a full one, e.g. TestController.cs \$\endgroup\$ – t3chb0t Apr 8 '18 at 9:11
  • \$\begingroup\$ The notification is sent in the Register method in AccountController. There are operations that are performed before and after in this method \$\endgroup\$ – user149517 Apr 8 '18 at 9:17
6
\$\begingroup\$

As you have already suspected, your code is trying to do too much. Consider separating functionality out into their own distinct concerns. (SRP/SoC -Single Responsibility Principle / Separation of Concerns)

Your abstract Notification acts more like a base model and should be refactor to reflect that

public abstract class Notification {

    public string To { get; }

    public abstract string Subject { get; }

    public abstract string Template { get; }

    public virtual object Model { get; }

    public Notification(string to, object model) {
        this.To = to;
        this.Model = model;
    }
}

The class is now responsible solely for storing the data to be sent.

This would mean that the RegisterConfirmationNotification class definition would now look like

public class RegisterConfirmationNotification : Notification {
    public RegisterConfirmationNotification(string to, object model)
        : base(to, model) {

    }

    public override string Subject => "Account Confirmation";

    public override string Template => "Views/EmailTemplates/RegisterConfirmation.cshtml";
}

SendAsync should be refactored into its own service abstraction and implementation and also follow explicit dependency principle by accepting a Notification argument directly

public interface INotificationService {
    Task SendAsync(Notification notification);
}

The implementation can be as simple as the following

public class NotoficationService : INotificationService {
    private readonly IEmailSender emailSender;

    public NotoficationService(IEmailSender emailSender) {
        this.emailSender = emailSender;
    }

    public async Task SendAsync(Notification notification) {
        var engine = new RazorLightEngineBuilder()
           .UseFilesystemProject(Directory.GetCurrentDirectory())
             .UseMemoryCachingProvider()
             .Build();

        string message = await engine.CompileRenderAsync(notification.Template, notification.Model);

        await emailSender.SendEmailAsync(notification.To, notification.Subject, message);
    }
}

Even the compilation of the razor template can be extracted out into its own service in order for this code to be more SOLID. I'll leave that choice up to you to implement.

Finally, the AccountController would now explicitly depend on the INotificationService in order to be able to send notifications. The Notification derived model can be then passed to the service and the message sent.

public class AccountController : Controller {
    private readonly INotificationService notificationService;

    public AccountController(INotificationService notificationService) {
        this.notificationService = notificationService;
    }

    //...

    [HttpPost]
    public async Task<IActionResult> Register([FromBody]MyModel model) {
        //...

        var notificationModel = new RegisterConfirmationViewModel { FirstName = "Jacques" };
        var notification = new RegisterConfirmationNotification (model.Email, notificationModel);
        await notificationService.SendAsync(notification);

        //...

        return View();
    }

}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy