HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Dependency injection with Factory pattern sample code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
samplewithfactorydependencycodeinjectionpattern

Problem

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:

-
Have I correctly implemented the Factory pattern?

-
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(); // Need  because of the Xml serializer.
        object DeserializeFromString();

    }

}


Factory

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

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

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

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


Serializers (Json in this example)

```
using Newtonsoft.Json;
using System;

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

Solution

public class SerializersFactory
{
    readonly ISerializer _iserializer;

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

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

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


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 ` type parameter here?

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


Oh, I see:

string SerializeToString(); // Need  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 deserialized);
    T Deserialize(string serialized); // 
}


And the implementation would be something like this:

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

    public T Deserialize(string serialized)
    {
        return JsonConvert.DeserializeObject(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.

Code Snippets

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>();
}
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);
}
string SerializeToString<T>(); // Need <T> because of the Xml serializer.
public interface ISerializer
{
    string Serialize<T>(T deserialized);
    T Deserialize<T>(string serialized); // <out T>
}

Context

StackExchange Code Review Q#106471, answer score: 6

Revisions (0)

No revisions yet.