0
\$\begingroup\$

For some time I am trying to make a mocking library for C# that mocks objects with just 1 line of code. I am using abstract factory pattern to know what is the object's type so that I can work with concrete factories.

I have the following Abstract factory class which I am not very happy with.

public class MockContext<T>
{
   public T CreateMockObject()
   {
      IFactory<T> factory = null;

     if (typeof(T).IsPrimitive || typeof(T) == typeof(string) || typeof(T) == typeof(decimal))
     {
        factory = new PrimitiveFactory<T>();
     }
     else if (typeof(T).IsArray)
     {
        factory = new ArrayFactory<T>();
     }
     else if (typeof(IEnumerable).IsAssignableFrom(typeof(T)))
     {
        factory = new CollectionFactory<T>();
     }
     else if (typeof(T).IsClass && typeof(T) != typeof(string))
     {
        factory = new ClassFactory<T>();
     }

      return factory.Create();
  }
 }
}

Which gets called by the following class

   public class Mocker
   {
      public T MockObject<T>()
      {
         MockContext<T> context = new MockContext<T>();
         T mockObject = context.CreateMockObject();

         return mockObject;
      }
   }

This abstract factory instantiates other factories depending on of the object.

Here is my class factory:

   public class ClassFactory<T> : IFactory<T>
   {
      public T Create()
      {
         T mockObject = (T)Activator.CreateInstance(typeof(T));
         IEnumerable<PropertyInfo> properties = typeof(T).GetProperties().Where(p => p.CanWrite);

         GenericMethodInvokerUtil method = new GenericMethodInvokerUtil();

         foreach (PropertyInfo property in properties)
         {
            if (property.PropertyType == typeof(T))
               throw new Exception("Circular properties are not supported!");

            method.InvokeMockObject<T>(property.PropertyType, property, mockObject);
         }

         return mockObject;
      }
   }

Here is my Primitive factory:

   public class PrimitiveFactory<T> : IFactory<T>
   {
      private readonly Dictionary<Type, IPrimitiveFactory<T>> _primitiveFactories = new Dictionary<Type, IPrimitiveFactory<T>>()
      {
         { typeof(string), new StringFactory() as IPrimitiveFactory<T> },
         { typeof(bool), new BooleanFactory() as IPrimitiveFactory<T> },
         { typeof(byte), new ByteFactory() as IPrimitiveFactory<T> },
         { typeof(sbyte), new SbyteFactory() as IPrimitiveFactory<T> },
         { typeof(char), new CharFactory() as IPrimitiveFactory<T> },
         { typeof(decimal), new DecimalFactory() as IPrimitiveFactory<T> },
         { typeof(double), new DoubleFactory() as IPrimitiveFactory<T> },
         { typeof(float), new FloatFactory() as IPrimitiveFactory<T> },
         { typeof(int), new IntFactory() as IPrimitiveFactory<T> },
         { typeof(uint), new UintFactory() as IPrimitiveFactory<T> },
         { typeof(long), new LongFactory() as IPrimitiveFactory<T> },
         { typeof(ulong), new UlongFactory() as IPrimitiveFactory<T> },
         { typeof(short), new ShortFactory() as IPrimitiveFactory<T> },
         { typeof(ushort), new UshortFactory() as IPrimitiveFactory<T> },
         { typeof(object), new ObjectFactory() as IPrimitiveFactory<T> },
      };

      public T Create()
      {
         if (_primitiveFactories.ContainsKey(typeof(T)))
         {
            IPrimitiveFactory<T> primitiveFactory = _primitiveFactories[typeof(T)];
            return primitiveFactory.Create();
         }

         throw new KeyNotFoundException($"The key of type: {typeof(T)} is not found.");
      }
   }

All primitive types has their own factories which is determined by the type of the object.

Array factory:

public class ArrayFactory<T> : IFactory<T>
   {
      public T Create()
      {
         var genericMethodInvoker = new GenericMethodInvokerUtil();
         Array array = Activator.CreateInstance(typeof(T), new object[] { RandomUtil.Instance.Next(1, 100) }) as Array;
         for (int i = 0; i < array.Length; i++)
         {
            Type arrayElementType = typeof(T).GetElementType();
            array.SetValue(genericMethodInvoker.InvokeMockObject<T>(arrayElementType), i);
         }

         return (T)Convert.ChangeType(array, typeof(T));
      }
   }

Collection factory

   public class CollectionFactory<T> : IFactory<T>
   {
      public T Create()
      {
         ICollectionFactory<T> factory = null;

         if (typeof(IDictionary).IsAssignableFrom(typeof(T)))
         {
            factory = new DictionaryFactory<T>() as ICollectionFactory<T>;
         }
         else if (typeof(IList).IsAssignableFrom(typeof(T)))
         {
            factory = new ListFactory<T>() as ICollectionFactory<T>;
         }

         return factory.Create();
      }
   }

Collections are on the other hand are separated in tow Dictionary and List

public class DictionaryFactory<T> : ICollectionFactory<T>
   {
      public T Create()
      {
         GenericMethodInvokerUtil methodInvoker = new GenericMethodInvokerUtil();
         IDictionary dictionary = (IDictionary)Activator.CreateInstance(typeof(T));
         int numberOfElements = RandomUtil.Instance.Next(1, 100);

         Type[] arguments = dictionary.GetType().GetGenericArguments();
         Type keyType = arguments[0];
         Type valueType = arguments[1];

         for (int i = 0; i < numberOfElements; i++)
         {
            object key = methodInvoker.InvokeMockObject<object>(keyType);
            object value = methodInvoker.InvokeMockObject<object>(valueType);

            if (!dictionary.Contains(key))
            {
               dictionary.Add(key, value);
            }
         }

         return (T)dictionary;
      }
   }

And List factory

public class ListFactory<T> : ICollectionFactory<T>
   {
      public T Create()
      {
         GenericMethodInvokerUtil methodInvoker = new GenericMethodInvokerUtil();
         IList list = (IList)Activator.CreateInstance(typeof(T));

         int numberOfElements = RandomUtil.Instance.Next(1, 100);
         Type valueType = list.GetType().GetGenericArguments()[0];

         for (int i = 0; i < numberOfElements; i++)
         {
            object value = methodInvoker.InvokeMockObject<object>(valueType);
            list.Add(value);
         }

         return (T)list;
      }
   }

I am using the above code as follows:

  static void Main(string[] args)
  {
     Mocker mocker = new Mocker();
     var mock = mocker.MockObject<Test>();
  }

With Test being some class with properties.

Mock object gets called recursively from the following utility class:

   public class GenericMethodInvokerUtil
   {
      public object InvokeMockObject<T>(Type type, PropertyInfo property, object currentObject)
      {
         object concreteResult = GetMockObjectResult(type);
         property.SetValue(currentObject, concreteResult);

         return currentObject;
      }

      public object InvokeMockObject<T>(Type type)
      {
         object concreteResult = GetMockObjectResult(type);
         return concreteResult;
      }

      private object GetMockObjectResult(Type type)
      {
         MethodInfo method = typeof(Mocker).GetMethod("MockObject", new Type[0] { });
         MethodInfo generic = method.MakeGenericMethod(type);

         object instance = Activator.CreateInstance(typeof(Mocker));
         object result = generic.Invoke(instance, null);
         object concreteResult = Convert.ChangeType(result, type);

         return concreteResult;
      }
   }

Can anyone suggest how to write this better?

Maybe there is a pattern that I am not familiar with. Any help would be appreciated.

You can find the complete source of the project in my github HERE.

\$\endgroup\$
9
  • 2
    \$\begingroup\$ This method is obviously a part of something bigger... could please add the rest of the picture too? It's hard to say anything seeing only this. A usage example would also be very helpful. \$\endgroup\$
    – t3chb0t
    Commented Jul 15, 2018 at 17:13
  • 1
    \$\begingroup\$ Sure. Just give me a moment. \$\endgroup\$ Commented Jul 15, 2018 at 17:15
  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$
    – Mast
    Commented Jul 15, 2018 at 17:35
  • 1
    \$\begingroup\$ @Mast I've updated the title... \$\endgroup\$
    – t3chb0t
    Commented Jul 15, 2018 at 17:39
  • 2
    \$\begingroup\$ @t3chb0t Alright, I will give more of the code and will include link to github. \$\endgroup\$ Commented Jul 15, 2018 at 17:58

1 Answer 1

3
\$\begingroup\$

Here're a couple of issues that struck me most...


Instead of writing lengthy if/else if/else trees you should use early returns with simple ifs. This will make your code cleaner and allow you put a nice throw at the end in case an unsupported type was specified.

public T CreateMockObject()
{
    if (typeof(T).IsPrimitive || typeof(T) == typeof(string) || typeof(T) == typeof(decimal))
    {
        return new PrimitiveFactory<T>().Create();
    }

    if (typeof(T).IsArray)
    {
        return new ArrayFactory<T>().Create();
    }

    ...

    throw new UnsupportedTypeException(..);
}

Currently your factories are separated from the decision ifs that pick them. It'd be easier to maintain this chaos if you created all factories before using them and added a new API like CanCreate<T> so that you can use a nice linq query to find a factory:

factories
    .SingleOrDefault(f => f.CanCreate<T>())
    ?.Create() ?? throw ...

The CanCreate<T> of each factory would implement the condition that are currently part of the ifs.


I find you should use var everywhere. It makes the code less verbose.


The name of the exception is a very important information source so you should create your own very specific exceptions that inform the user what exactly went wrong.

throw new Exception("Circular properties are not supported!");

This e.g. should be replaced by CircularPropertyException and its message should contain the name of the affected property. Currently this exception is not helpful at all. You know that something went wrong but you don't know what.


There are too many hardcoded values like:

RandomUtil.Instance.Next(1, 100)

The user should have a choice what to create. Giving him a random array makes tests using such mocks completely unpredictable because you'll get different data each time.


I'm not entirely sure what this framework does because of the issues I mentioned. You should clean it up and add more inline comments explaining why you do certain things and maybe post a follow-up. E.g. I could not tell what the ClassFactory is supposed to do. I know it instantiates T but what happens after that is a complete mystery to me. You should either better comment this code or move some parts into other methods so that their names at least give some hints about the ins and outs.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ I greatly value your opinion. You have made some pretty good points. I especially liked the CanCreate method and definitely going to implement that one first. The library is in a very early stage. I am aware about the hardcoded values which of course would be configurable. The same thing is with the custom exception. I just wanted to see if I am in the right track here. Thank you for detailed explanation. \$\endgroup\$ Commented Jul 17, 2018 at 19:26
  • 1
    \$\begingroup\$ @HasanHasanov When you're done with the clean-up and if you decide to post a follow-up, make sure to add some usage examples. It's always easier to figure out how something works this way - happy refactoring ;-) \$\endgroup\$
    – t3chb0t
    Commented Jul 17, 2018 at 19:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.