cd491415
cd491415

Reputation: 891

Unit Testing Factory/Service Locator - Static class

Recently, I saw this code. Since I am trying to learn few things at once, I am having issues with this code but dont know how to resolve. I would like to be able to unit test this code

public static class CarFactory
{
    private static readonly IDictionary<string, Type> CarsRegistry = new Dictionary<string, Type>();

    public static void Register<TCar>(string car) where TCar : CarBase, new()
    {
        if (!CarsRegistry.ContainsKey(car))
        {
            CarsRegistry.Add(car, typeof(TCar));
        }
    }

    public static ICar Create(string car)
    {
        if (CarsRegistry.ContainsKey(car))
        {
            CarBase newCar = (CarBase)Activator.CreateInstance(CarsRegistry[car]);
            return newCar;
        }

        throw new NotSupportedException($"Unknown '{car}'");
    }
}

I have few problems with this code.

  1. Name is CarFactory but this does not look like a Factory Pattern to me. It looks more like Locator Pattern
  2. The class is static - and I heard static classes are bad for unit testing in frameworks like Moq and that they also hide dependencies. Say a method in another regular class uses this, for unit test, there is no way to know that that method depend on this static class

I would like to ensure this class is called properly and I think based on my reading that this is Locator Pattern.

I would also like to unit test this class and need help to make it unit testable using Moq.

Thanks to @ErikPhillips explanation below, I now understand that other classes using this class will be not testable. So, if I have a class like below:

public class CarConsumer
{
   public void ICar GetRedCar()
   {
     var result = CarFactory.Create("Tesla");
     result.Color = Color.Red;
     return result;
   }
}

, GetRedCar() method will be difficult to test because it uses CarFactory static class and for a unit test or an outside client, there is nothing in GetRedCar() method API that suggest it is depending on this static class.

I would like to refactor CarFactory class so other classes using it like in example above CarConsumer class can be tested properly.

Upvotes: 0

Views: 510

Answers (1)

Erik Philips
Erik Philips

Reputation: 54638

I would like to be able to unit test this code

What specific problems prevent you from unit testing this class? It has two methods, seems pretty straight forward to write a unit test against.

Name is CarFactory but this does not look like a Factory Pattern to me

I believe a Factory Pattern is

The factory method pattern is a creational pattern that uses factory methods to deal with the problem of creating objects without having to specify the exact class of the object that will be created

I pass in the name of the car (so I haven't specified the type) and it creates the class for me. That's pretty much it. Is it a good example of one? Not in my opinion, but my opinion of how well it's done doesn't change what it is.

This doesn't mean it's not Service Locator, but it definitely is a Factory Method. (Honestly it doesn't look like a service locator because it only delivers a single service)

unit testing in frameworks like Moq

Moq is not a Unit-Testing framework. Moq is a Mocking Framework. Static classes are not easy to Mock. If you can Mock it, you can unit-test with methods that require a mocked class.

static classes .. that they also hide dependencies.

Any poorly designed anything can do anything. Static Classes aren't by definition designed to hide anything.

I would say in this instance, that this Static Class, prevents you from being able to Mock it easily to unit test other methods that rely on the Static Classes methods.

I would also like to unit test this class and need help to make it unit testable using Moq.

Again, there is nothing preventing you from unit-testing this class.

public class CarFactoryTests
{  
  public class MoqCar : CarBase { }

  public void Register_WithValidParameters_DoesNotThrowException
  {
    // Act
    Assert.DoesNotThrow(() => CarFactory.Register<MoqCar>(
      nameof(Register_WithValidParameters_DoesNotThrowException)));
  }

  public void Create_WithValidCar_DoesNotThrowException
  {
    CarFactory.Register<MoqCar>(
      nameof(Create_WithValidParameters_DoesNotThrowException));

    Assert.DoesNotThrow(() => CarFactory.Create(
      nameof(Create_WithValidParameters_DoesNotThrowException));
  }

  // etc etc
}

The problem you may run into is

public class CarConsumer
{
   public void ICar GetRedCar()
   {
     var result = CarFactory.Create("Tesla");
     result.Color = Color.Red;
     return result;
   }
}

Testing this method means that you are not in full control of the method because there is external code GetRedCar() relies on. You cannot write a pure unit test here.

This is why you'd have to convert CarFactory to an instance class. And then make sure it's has the correct lifetime for whatever DI framework you're using.

public class CarConsumer
{
   private ICarFactory _carFactory;
   public CarConsumer(ICarFactory carFactory)
   {
     _carFactory = carFactory;
   }

   public void ICar GetRedCar()
   {
     var result = _carFactory.Create("Tesla");
     result.Color = Color.Red;
     return result;
   }
}

Now we can Moq ICarfactory and write pure unit test against GetRedCar().

The following is not recommended.

If for whatever reason you are stuck with this type of Factory but you still want to write pure unit tests, you can do something like:

public class CarConsumer
{
   private Func<string, ICar> _createCar;
   public CarConsumer(Func<string, ICar> createCar= CarFactory.Create)
   {
     _createCar = createCar;
   }

   public void ICar GetRedCar()
   {
     var result = _createCar("Tesla");
     result.Color = Color.Red;
     return result;
   }
}

We can Moq this type of Func, but it's really just a crutch for the real problem.

I guess the real question I have is how to make my CarFactory so that methods from other classes using it can be tested using Moq?

public interface ICarFactory
{
  void Register<TCar>(string car) where TCar : CarBase, new();
  ICar Create(string car);
}

public class CarFactory : ICarFactory
{
  private readonly IDictionary<string, Type> CarsRegistry 
    = new Dictionary<string, Type>();

  public void Register<TCar>(string car) where TCar : CarBase, new()
  {
    if (!CarsRegistry.ContainsKey(car))
    {
      CarsRegistry.Add(car, typeof(TCar));
    }
  }

  public ICar Create(string car)
  {
    if (CarsRegistry.ContainsKey(car))
    {
      CarBase newCar = (CarBase)Activator.CreateInstance(CarsRegistry[car]);
      return newCar;
    }

    throw new NotSupportedException($"Unknown '{car}'");
  }
}

Upvotes: 4

Related Questions