Thursday, October 31, 2013

Fixing Interface Segregation Principle Violations And Applying Dependency Injection Using Unity

Following SOLID principles in object oriented design is a good thing. It allows you to produce more maintainable and testable code.

Let's look at a common situation when the "I" letter principle in SOLID acronym is violated. Assume you have an application that reads various settings from different places (app.config, database, service etc.). For that purpose one can create a class that encapsulates all settings access logic along with some data type conversions. The class is then injected as a dependecy according to Dependency Inversion principle (letter "D" in SOLID acronym). Here is an example that is written on C# and uses Unity as an IoC container:

public static void Main()
{
 var container = new UnityContainer();
 container.RegisterType<ISiteSettings, SiteSettings>();
 container.RegisterType<EmailSender>();
 container.RegisterType<TitlePrinter>();
 container.RegisterType<ApplicationCache>();

 container.Resolve<EmailSender>().SendEmail();
 container.Resolve<TitlePrinter>().Print();
 container.Resolve<ApplicationCache>().Insert();
 Console.ReadKey();
}

First we register the settings and some helpers which use them: EmailSender, TitlePrinter and ApplicationCache. Right after the registration we resolve helpers and trigger their methods. Our IoC container injects the settings automaticaly so we don't have to resolve them and pass to helper classes manually. Here is what we have so far for settings:

public class SiteSettings : ISiteSettings
{
 public SiteSettings()
 {
  Console.WriteLine("----------New Site settings instance created------------");
 }

 public int CacheTimeoutMinutes
 {
  get { return 1; }
 }

 public string Title
 {
  get { return "My awesome site"; }
 }

 public string EmailSenderName
 {
  get { return "Vasya"; }
 }

 public string EmailSenderAddress
 {
  get { return "vasya@domain.com"; }
 }
}

public interface ISiteSettings
{
 int CacheTimeoutMinutes { get; }

 string Title { get; }

 string EmailSenderName { get; }

 string EmailSenderAddress { get; }
}

The helpers are all basicaly the same so I'm going to put only EmailSender here:

public class EmailSender
{
 private readonly ISiteSettings m_settings;

 public EmailSender(ISiteSettings settings)
 {
  m_settings = settings;
 }

 public void SendEmail()
 {
  Console.WriteLine(
   "Email is sent by {0} from {1}",
   m_settings.EmailSenderName,
   m_settings.EmailSenderAddress);
 }
}

The settings are injected via constructor and stored in m_settings field. I'm sure constructors are the best place to perform injection because of three things:
1. There is no way of missing a dependency as you have to explicitly specify all of them during creation.
2. It is clear for the class consumer what this class relies on.
3. If you find yourself using huge constructors it's time to refactor the class in order to follow Single Responsibility principle (letter "S" in SOLID acronym).
 Once we call SendEmail method we simply print a message based on m_settings. It's just an example and in a real app you would place some valuable code there. Let's look at the output:

----------New Site settings instance created------------
Email is sent by Vasya from vasya@domain.com
----------New Site settings instance created------------
My awesome site
----------New Site settings instance created------------
Item inserted into cache. Duration is set to 1

Good news is that the app works. However we have a few problems here. First of all ISiteSettings interface contains all application settings and helper classes are forced to pull all its members even if they don't need all of them. This is a typical violation of Interface Segregation principle and the code quickly becomes messy as application grows. Second we create a separate instance of SiteSettings on every injection which is redundant.

In order to address the first problem let's group all the settings by their purpose and split the ISiteSettings interface into three smaller ones:

public interface IAppearanceSettings
{
 string Title { get; }
}
 
public interface ICacheSettings
{
 int CacheTimeoutMinutes { get; }
}

public interface IEmailSettings
{
 string EmailSenderName { get; }

 string EmailSenderAddress { get; }
}

public class SiteSettings : IEmailSettings, ICacheSettings, IAppearanceSettings
{
 ...
}

Now for every helper class we have to provide only what it really needs.

public class EmailSender
{
 private readonly IEmailSettings m_settings;

 public EmailSender(IEmailSettings settings)
 {
  m_settings = settings;
 }

 public void SendEmail()
 {
  Console.WriteLine(
   "Email is sent by {0} from {1}",
   m_settings.EmailSenderName,
   m_settings.EmailSenderAddress);
 }
}

We've just fixed Interface Segregation principle violation. However we still create an instance of SiteSettings class every time we resolve a dependency. One possible way of solving the problem is to create a separate instance of SiteSettings and specify it during registration phase like this:

var container = new UnityContainer();
SiteSettings settings = new SiteSettings();
container.RegisterType<IEmailSettings>(new InjectionFactory(i => settings));
container.RegisterType<IAppearanceSettings>(new InjectionFactory(i => settings));
container.RegisterType<ICacheSettings>(new InjectionFactory(i => settings));

This will work. However you have to create the instance at the very begining of your application lifetime. In some circumstances it might not be a desirable option (for example if you wan't to perform a lazy initialization). In this case you could use the following approach:

var container = new UnityContainer();
container.RegisterType<SiteSettings>(new CustomLifetimeManager());
container.RegisterType<EmailSender>(new InjectionConstructor(new ResolvedParameter<SiteSettings>()));
container.RegisterType<TitlePrinter>(new InjectionConstructor(new ResolvedParameter<SiteSettings>()));
container.RegisterType<ApplicationCache>(new InjectionConstructor(new ResolvedParameter<SiteSettings>()));

Pay attention for SiteSettings registration as we pass an instance of CustomLifetimeManager there. Also we have to explicitly tell IoC container that we wan't to inject SiteSettings instance to our helpers. By default Unity tries to find registrations for IEmailSettings, IApprearanceSettings and ICacheSettings and fails (as we did not register anything with these types). Here is what CustomLifetimeManager looks like:

public class CustomLifetimeManager : LifetimeManager
{
 private object m_value;

 public override object GetValue()
 {
  return m_value;
 }

 public override void SetValue(object newValue)
 {
  m_value = newValue;
 }

 public override void RemoveValue()
 {
  m_value = null;
 }
}

We use m_value field to save SiteSettings instance and then return it when necessary. In web apps we could store the value somewhere else (in HttpContext for example). And here is the final console output:

----------New Site settings instance created------------
Email is sent by Vasya from vasya@domain.com
My awesome site
Item inserted into cache. Duration is set to 1

All the code mentioned above can be found in my github repository. Hope this helps.

1 comment:

  1. Thanks for the article. I will definitely use this method in our projects. ☺

    ReplyDelete