Dependency injection with Factory pattern sample code

Question

I'm trying to understand and learn about the SOLID principle and especially Dependency Injection (DI) and Factory pattern.

Some background:

I have a production application that uses some "helpers" classes but those are all-in-one classes that I just copy into each new project. One of the (most used) functions is serialization. Our customer has strict demands about history/revision tracing so all important requests are serialized to XML and inserted in Sql server.

With SOLID, DI and Factory in mind I decided to rewrite ye old helpers into a fresh C# project and do things properly.

Below is sample code for Serializers (I have three, XML, JSON and Binary), shown is only the Json class but they are all pretty much the same.

My questions are:

1. Have I correctly implemented the Factory pattern?

2. Is the Dependency Injection the way it is supposed to be?

I'm not exactly sure if I even covered both patterns, a mix or perhaps just one?

I hope I made things clear, if not let me know, and thanks for helping me understand DI/Factory better.

Interface

namespace nUtility.Serializers
{
    public interface ISerializer
    {
        string InputString { get; set; }
        object InputObject { get; set; }

        string SerializeToString<T>(); // Need <T> because of the Xml serializer.
        object DeserializeFromString<T>();

    }

}

Factory

namespace nUtility.Serializers
{
    public class SerializersFactory
    {
        readonly ISerializer _iserializer;

        public SerializersFactory(ISerializer iserializer)
        {
            _iserializer = iserializer;
        }

        public string Serialize<T>() => _iserializer.SerializeToString<T>();

        public object Deserialize<T>() => _iserializer.DeserializeFromString<T>();
    }
}

Serializers (Json in this example)

using Newtonsoft.Json;
using System;

namespace nUtility.Serializers
{
    public class nSerializerJson : ISerializer
    {
        public string InputString { get; set; }
        public object InputObject { get; set; }

        public nSerializerJson(string inputString, object inputObject)
        {
            if (inputString == null)
                throw new ArgumentNullException(nameof(inputString));

            if (inputObject == null)
                throw new ArgumentNullException(nameof(inputObject));

            InputObject = inputObject;
            InputString = inputString;

        }

        public string SerializeToString<T>()
        {
            return JsonConvert.SerializeObject(InputObject);
        }

        public object DeserializeFromString<T>()
        {
            return JsonConvert.DeserializeObject<T>(InputString);
        }

    }
}

Program.cs

(SampleValues.InputObjectList is a simple test class which populates a List with 3 items)

var serializersFactory = new SerializersFactory(new nSerializerJson(expectedString, SampleValues.InputObjectList));
expectedString = serializersFactory.Serialize<List<SerializeTestObject>>();

Two more things:

Why not static class for serializers - I kinda agree with this answer on SO, however on a second thought changing my class (the parameter creep/constructor point in SO answer) would violate SOLID (the SRP) so I'm not sure if this is correct decision?

I know I'm breaking the Type and Namespace naming convetion by putting "nSerializers" etc. but this "n" prefix has been in the company for 9 years now and we're not prepared to let go yet :).

Answer 1

public class SerializersFactory
{
    readonly ISerializer _iserializer;

    public SerializersFactory(ISerializer iserializer)
    {
        _iserializer = iserializer;
    }

    public string Serialize<T>() => _iserializer.SerializeToString<T>();

    public object Deserialize<T>() => _iserializer.DeserializeFromString<T>();
}

Sorry to burst your bubble, but... this isn't a factory. In fact, it's rather confusing what purpose it serves, given how that serializer is implemented:

    public nSerializerJson(string inputString, object inputObject)
    {
        if (inputString == null)
            throw new ArgumentNullException(nameof(inputString));

        if (inputObject == null)
            throw new ArgumentNullException(nameof(inputObject));

        InputObject = inputObject;
        InputString = inputString;
    }

It's very confusing why you need to specify both the serialized JSON string and the deserialized object for the constructor to not throw an ArgumentNullException. Actually, it's very hard to understand why one would even need this constructor.

It's the methods that need the parameters; having them at instance-level seems to make the whole thing more or less useless - and confusing.

What's the role of the <T> type parameter here?

public string SerializeToString<T>()
{
    return JsonConvert.SerializeObject(InputObject);
}

Oh, I see:

string SerializeToString<T>(); // Need <T> because of the Xml serializer.

Since when do implementations dictate what abstractions look like? That T is clearly begging for a method parameter there, of type T.

---

Seems you need to stop and ask yourself what you're trying to accomplish, and why. Your system has a dependency on Newtonsoft.Json; your serializer is essentially wrapping the Newtonsoft API... in some awkward way.

I would have used something like this:

public interface ISerializer
{
    string Serialize<T>(T deserialized);
    T Deserialize<T>(string serialized); // <out T>
}

And the implementation would be something like this:

public class JsonSerializer : ISerializer
{
    public string Serialize<T>(T deserialized)
    {
        return JsonConvert.SerializeObject(deserialized);
    }

    public T Deserialize<T>(string serialized)
    {
        return JsonConvert.DeserializeObject<T>(serialized);
    }
}

What else would you need? I think everything else you got here, is over-engineered fluff that can be removed. There's nothing to DI here, ISerializer is the dependency.

As for the factory, there's no need for one, since any type that needs an ISerializer can just tell the IoC container it needs one, by specifying an ISerializer parameter in its constructor.