I am using the excellent Simple Injector Ioc framework and would like to "plug in" multiple email output services ie Mandrill, MailChimp etc My question really is am I doing this correctly as it results in a Cast at my send method.
So I have a simple IEmailOutputService
public interface IEmailOutputService
{
string Identifier { get; }
bool Send(object message, object contents);
}
And a MandrillOutputService (shortened)
public class MandrillOutputService : IEmailOutputService
{
public MandrillOutputService()
{
//DI stuff here
}
public string Identifier => "Mandrill";
public bool Send(EmailMessage message, IEnumerable<TemplateContent> templateContents)
{
if (message == null)
return false;
//send code here
return true;
}
bool IEmailOutputService.Send(object message, object contents)
{
//TODO this doesnt look right!!
var m = message as EmailMessage;
var c = contents as IEnumerable<TemplateContent>;
//forwards method onto bespoke Mandrill Send method above
return Send(m, c);
}
}
I have an EmailContext
that gets the Email Output Provider for the logged in User eg "Mandrill" heres the IEmailContext
public interface IEmailContext
{
string GetProvider();
}
EmailOutputComposite is used to select correct Email Output Service
public class EmailOutputComposite : IEmailOutputService
{
private readonly IEmailContext _emailContext;
private readonly IEnumerable<IEmailOutputService> _emailOutputServices;
public string Identifier => "EmailOutputComposite";
public EmailOutputComposite(
IEmailContext emailContext, IEnumerable<IEmailOutputService> emailOutputServices)
{
this._emailContext = emailContext;
this._emailOutputServices = emailOutputServices;
}
bool IEmailOutputService.Send(object message, object contents) =>
this._emailOutputServices
.FirstOrDefault(x => x.Identifier.ToLower() == this._emailContext.GetProvider())
.Send(message, contents);
}
and finally registrations in Simple Injector
container.RegisterCollection(typeof(IEmailOutputService), new[]
{
typeof(MandrillOutputService)
//other emailOutputServices to go here
});
container.Register(typeof(IEmailOutputService), typeof(EmailOutputComposite),
Lifestyle.Singleton);
So my question is am I doing this correctly or is there a better way. I have to get the Users Email Provider (Mandrill) from Database so cant think of another way to do this but was concerned with the Cast I have to do in MandrillOutputService.Send method.
I see two problems in your design:
MandrillOutputService
by accepting only a subset of the accepted types of the IEmailOutputService
abstraction; this might cause the appliation to break at runtime when the user supplies values that are invalid for that specific implementation.Identifier
property on the IEmailOutputService
violates the Interface Segration Principle, because it is a method that consumers don't use. The only class that is actually interested in this property is the EmailOutputComposite
. Removing the Identifier
from the abstraction has the advantage that it can simplify unit testing, since there is less code that a consumer can call. It also simplifies the interface, which is always a good thing.I'm unsure how to fix the LSP principle, because its unclear to me how other implementations look like.
With respect to the ISP violation, you can do the following to fix it:
Identifier
. This allows you to remove the property from the interface, but the downside is that the Composite can only filter those services in case the actual types are injected (and not decorated, because that disallows you from retrieving those attributes).switch
-case
statement inside the Composite. This again allows you to remove the property from the interface, but downside is that you will have to update the composite every time a new implementation is added (which might not be that bad if you consider the Composite part of your Composition Root).Identifier
is the dictionary's key. This removes the need to have the Identifier
as part of the abstraction, but also removes the identifier from the implementation (which might actually be something good).Here's an example of this last example:
container.RegisterSingleton<IEmailOutputService, EmailOutputComposite>();
container.RegisterSingleton(new Dictionary<string, Func<IEmailOutputService>>()
{
"Mandrill", CreateEmailServiceProducer<MandrillOutputService>(container),
"other", CreateEmailServiceProducer<Other>(container),
// ..
});
privte static Func<IEmailOutputService> CreateEmailServiceProducer<T>(Container c)
where T : IEmailOutputService =>
Lifestyle.Transient.CreateProducer<IEmailOutputService, T>(c).GetInstance;
Where the Composite is implemented as follows:
public class EmailOutputComposite : IEmailOutputService
{
private readonly IEmailContext _emailContext;
private readonly Dictionary<string, Func<IEmailOutputService>> _emailOutputServices;
public EmailOutputComposite(
IEmailContext emailContext,
Dictionary<string, Func<IEmailOutputService>> emailOutputServices)
{
_emailContext = emailContext;
_emailOutputServices = emailOutputServices;
}
public bool Send(object m, object c) => Service.Send(m, c);
IEmailOutputService Service => _emailOutputServices[_emailContext.GetProvider()]();
}
Whether or not this is actually an improvement is up to you.