Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SerializerBuilder interface for IoC #517

Open
sonic1981 opened this issue Aug 7, 2020 · 5 comments
Open

SerializerBuilder interface for IoC #517

sonic1981 opened this issue Aug 7, 2020 · 5 comments

Comments

@sonic1981
Copy link

Looking at the SerializerBuilder class this seems to be screaming out for an interface. This would allow for better IoC utilisation, particularly when wanting to mock dependanices for unit testing:

private readonly ISerializerBuilder serializerBuilder;

public Class(ISerializerBuilder serializerBuilder)
{
   this.serializerBuilder = serializerBuilder;
}

public void Usage()
{
   //this already returns a nice interface
   this.serializerBuilder.Build();
}

It would be trivial to add?

@aaubry
Copy link
Owner

aaubry commented Aug 7, 2020

Why would you want to inject a SerializerBuilder ? Why not inject an ISerializer instead ?

@sonic1981
Copy link
Author

sonic1981 commented Aug 7, 2020

Well, what's the role of the SerializerBuilder? I'm guessing this is a factory pattern? If I just inject ISerializer why do I need a builder at all? Why not just new up one of these directly?

@sonic1981
Copy link
Author

sonic1981 commented Aug 7, 2020

Looking at the source it looks like it essentially exists to decouple the Serializer from it's dependancies, like CachedTypeInspector. Is that fair to say?

I guess the issue with not being able to inject is this where you want different functionality in different consumers. So, as an example and picking a method at random, one class wants a serializer with TagMapping and one doesn't. Then you have to register multiple serializers and start tightly coupling the implementation to a particular IoC. Where as if you could just inject the factory you could new up new ones however you want and also have good IoC and unit tests, etc.

@aaubry
Copy link
Owner

aaubry commented Aug 7, 2020

The serializer is just an aggregator of many different components that are composed together. The SerializerBuilder allows you to configure the serializer without needing to know all the details of that composition, but its interface is coupled to the serializer implementation, so there's no point on having an abstraction for it.

I believe that the use case that you describe is far from common. I think you would be better served to define your own factory interface with methods that make sense for your use case.

@EdwardCooke
Copy link
Collaborator

If I recall the serializerbuilder and serializer are both not thread safe so you would need to scope them as transient object. I understand your desire for an interface around them, which I’m pretty sure serializer does. If we made the methods virtual instead of needing to maintain and interface would that suffice?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants