# Abstracting an Email Notification Service & testing the the logic of the used abstract factory

In my application I have an Interface IEmailNotification that represents an Email and the concrete implementations have nothing to do with MailMessage class so to be able to send it through SmtpClient I used an adapter to let the communication between SmtpClient and IEmailNotification possible and the job of this adapter is simply delegate the Send call to SmtpClient but after converting from EmailNotification to MailMessage using an abstract Factory which in fact only copying the values from one object to the new created MailMessage object.

The first question is: my factory method takes a parameter of type IEmailNotification, does that violates the main Job of a Factory which is only creating objects?

public interface INotification
{
string To { get; }
string Body { get; }
}

{
string From { get; }
string Subject { get; }
bool IsBodyHtml { get; }
string CC { get;  }
string BCC { get;  }
List<string> AttachmentsPaths { get; }
}

{
}

{
private ISmtpClient _client;
private MailMessageFactory _mailMessageFactory;

{
_client = client;
_mailMessageFactory = mailMessageFactory;
}

public void Dispose()
{
_client.Dispose();
}

{
{
_client.Send(mailMessage);
}
}
}

public class EmailMailMessageFactory : MailMessageFactory
{
public EmailMailMessageFactory(string backupBccEmail)
: base(backupBccEmail)
{
}

{
using (var mailMessage = new MailMessage())
{

{
{
}
}

return mailMessage;
}
}
}


backupBccEmail is required for manipulating the EmailNotification based on some logical conditions

The second question is: When I tried to test the logic of this method It failed because it uses an external resource at the line of adding new attachments where it cannot find the related paths on the Hard Drive, what do you people think about it?

Unit Test Note: I know that only one assert per test is recommended but for now let's keep it simple & straightforward

[TestFixture]
public class EmailMailMessageFactoryTests
{

[SetUp]
public void SetUp()
{
{
AttachmentsPaths = new List<string> { "1", "2" },
CC = "cc@test.com",
BCC = "bcc@test.com"
};
}

[Test]
public void CreateMailMessage_WhenCalled_CreatesMailMessage()
{
var emailMailMessageFactory = new EmailMailMessageFactory("backup@test.com");

//this assert fails "FileNotFoundException"

}


}

Maybe I'm misusing what's called Factory Pattern in my code!

• Please post sufficient context to help us review your code. Include code for IEmailNotification and redirectToEmail. Also show us how you use this code through a trivial unit test. – dfhwze Jul 17 at 9:29
• and be cautious calling something an abstract factory .. this is a different pattern altogether en.wikipedia.org/wiki/Abstract_factory_pattern – dfhwze Jul 17 at 9:33
• @dfhwze "be cautious calling something an abstract factory" that's why I have doubts about it, I'll edit the question – Rasheed Jul 17 at 9:38
• I couldn't come up with a better title, I hope this is enough! – Rasheed Jul 17 at 11:05

You are disposing the message before it even has a chance to be used by a consumer.

Remove the using block in the factory method

public override MailMessage CreateMailMessage(IEmailNotification emailNotification) {
var mailMessage = new MailMessage();

foreach (var path in emailNotification.AttachmentsPaths) {
}
}

return mailMessage;
}


Leave the responsibility of disposal to the consumer of the factory.

my factory method takes a parameter of type IEmailNotification, does that violates the main Job of a Factory which is only creating objects?

A factory method can take explicit dependencies which it can use to perform its required functionality.

When I tried to test the logic of this method It failed because it uses an external resource at the line of adding new attachments where it cannot find the related paths on the Hard Drive, what do you people think about it?

Implementation concerns should be encapsulated behind abstractions that avoid tight coupling to external dependencies.

In this case, when you were testing, the Attachment will try to read the file at the provided path. Since those paths may not exist when testing, you should consider refactoring the current design.

Provide an abstraction that would allow the attachment stream to be read in isolation without any adverse behavior.

public interface IFileInfo {
string Name { get; }
string PhysicalPath { get; }
}


Here is a simple implementation that can be used at run-time

public class AttachmentInfo : IFileInfo {

public AttachmentInfo(string path) {
innerFile = new FileInfo(path);
}

public string Name => innerFile.Name;

public string PhysicalPath => innerFile.FullName;

}


The email notification can be refactored to use the abstraction for attachments

public interface IEmailNotification : INotification {
string From { get; }
string Subject { get; }
bool IsBodyHtml { get; }
string CC { get; }
string BCC { get; }
List<IFileInfo> Attachments { get; }
}


Resulting in the factory method to become

public class EmailMailMessageFactory : MailMessageFactory {

public EmailMailMessageFactory(string backupBccEmail)
: base(backupBccEmail) {
}

var mailMessage = new MailMessage {
};

foreach (var file in emailNotification.Attachments) {
string filename = file.Name;
var attachment = new Attachment(stream, filename);
}
}
return mailMessage;
}
}


When testing in isolation, a fake stream can be given to the attachment to allow the subject under test to be exercised.

[TestClass]
public class EmailMailMessageFactoryTests {
[TestMethod]
public void CreateMailMessage_WhenCalled_CreatesMailMessage() {
//Arrange
var stream = new MemoryStream();
var attachments = new List<IFileInfo> {
Mock.Of<IFileInfo>(_ => _.Name == "1" && _.CreateReadStream() == stream)
};
_.From == "from@test.com" &&
_.To == "to@test.com" &&
_.Subject == "subject" &&
_.Body == "body" &&
_.IsBodyHtml == true &&
_.CC == "cc@test.com" &&
_.BCC == "bcc@test.com" &&
_.Attachments == attachments
);
var emailMailMessageFactory = new EmailMailMessageFactory("backup@test.com");

//Act

//Assert