Fashion of patterns

No, I will not talk about fashion but design patterns. If you think they are something great and that all software developers should know them, we agree, but I think that there is a small problem with them. There are many people who know them and don’t know very well how and when to apply them, so they make a poor implementation that does not solve any problems but complicates the application code. It’s nice to say: I have used the Singleton pattern, but do we know when we should use it or we just use it because it is nice?
 
Ten years ago I was working in a team of software architects from several countries performing the technical design of a corporate application for a large multinational. A French architect wanted to use all known design patterns because according to him they were the best practices recommended by Microsoft and a good application had to implement them all. It was difficult for me to convince him that design patterns solve specific problems and do not necessarily apply to all applications. There are still many people who still think that an application is better or worse depending on how many design patterns implement, and personally I think it is wrong.
 
This week I’ve been working in a project by reviewing the source code and I found curious things, as a Singleton pattern of an object that returns the connection to a web service. The idea is good, but not so much implementation. The object did not have state or stored the connection for reuse, so the Singleton pattern lost all its meaning. It had only a method that created and returned the connection. It was something like this:

public class ServiceProvider
{
  private static ServiceProvider _current = new ServiceProvider();
  public static ServiceProvider Current { get { return _current; } }

  public IService GetConnection()
  {
    ClientCredentials credentials = new ClientCredentials();
    if (ConfigurationSettings.AppSettings["username"] != null)
      credentials.Windows.ClientCredential = new NetworkCredential(
          ConfigurationSettings.AppSettings["username"],
          ConfigurationSettings.AppSettings["password"],
          ConfigurationSettings.AppSettings["domain"]);
    else
      credentials.Windows.ClientCredential =
          CredentialCache.DefaultNetworkCredentials;

    Uri ServiceUri = new Uri(ConfigurationSettings.AppSettings["ServiceUri"]);
    ServiceProxy serviceProxy = new ServiceProxy(ServiceUri, null, credentials, null);
    IService service = (IService)serviceProxy;
    return service;
  }
}

The connection could be stored as a private variable may to return it in subsequent calls rather than create it again, and then it would have sense the use of the Singleton pattern. To make implementation worse, when the object is called the Current property is not used to access the instance of the Singleton object, but rather it is created a new one each time.

var service = new ServiceProvider();
var connection = service.GetConnection();

Another problem with this implementation is that this connection object is passed as parameter to some methods, but in others is not passed as a parameter and instead the ServiceProvider object is created again inside the function and the method GetConnection is called for a new connection.
 
What would have been the good implementation? Well, the Singleton pattern is used when we want to have a single instance of an object and reuse it from all methods in our application. The reason why we would like to reuse an object is that it contains some state or resources (such as a connection to a web service) that we want to share. A static method or a static (such as Current or Instance) property allow us to access the instance without creating the object. In addition, an important feature of the Singleton pattern is that the creation of the object with new is not allowed, so we can set the object constructor as private. A possible implementation using the Singleton pattern could be as follows:

public class ServiceProvider
{
  private IService _service = null;
  private static ServiceProvider _current = new ServiceProvider();
  public static ServiceProvider Current { get { return _current; } }

  //Constructor is private
  private ServiceProvider()
  {
  }

  public IService GetConnection()
  {
    if (_service == null)
    {
      //Get credentials
      ClientCredentials credentials = new ClientCredentials();
      if (ConfigurationSettings.AppSettings["username"] != null)
        credentials.Windows.ClientCredential = new NetworkCredential(
            ConfigurationSettings.AppSettings["username"],
            ConfigurationSettings.AppSettings["password"],
            ConfigurationSettings.AppSettings["domain"]);
      else
        credentials.Windows.ClientCredential =
            CredentialCache.DefaultNetworkCredentials;

      //Create connection
      Uri ServiceUri = new Uri(ConfigurationSettings.AppSettings["ServiceUri"]);
      ServiceProxy serviceProxy = new ServiceProxy(ServiceUri, null, credentials, null);
      _service = (IService)serviceProxy;
    }
    return _service;
  }
}

In every function that uses the connection we could use the following lines:

IService connection = ServiceProvider.Current.GetConnection();

Although in this case I would opt for a simpler implementation:

public static class ServiceProvider
{
  private static IService _service = null;

  //Constructor is private
  private ServiceProvider()
  {
  }

  public static IService GetConnection()
  {
    if (_service == null)
    {
      //Get credentials
      ClientCredentials credentials = new ClientCredentials();
      if (ConfigurationSettings.AppSettings["username"] != null)
        credentials.Windows.ClientCredential = new NetworkCredential(
            ConfigurationSettings.AppSettings["username"],
            ConfigurationSettings.AppSettings["password"],
            ConfigurationSettings.AppSettings["domain"]);
      else
        credentials.Windows.ClientCredential =
            CredentialCache.DefaultNetworkCredentials;

      //Create connection
      Uri ServiceUri = new Uri(ConfigurationSettings.AppSettings["ServiceUri"]);
      ServiceProxy serviceProxy = new ServiceProxy(ServiceUri, null, credentials, null);
      _service = (IService)serviceProxy;
    }
    return _service;
  }
}

The call would be a little easier:

IService connection = ServiceProvider.GetConnection();

You can get more information on the Singleton pattern, hot it resolves problems and examples of implementation, in the Wikipedia article: http://en.wikipedia.org/wiki/Singleton_pattern