Search code examples
c#oopdependency-injectioncompositionsingle-responsibility-principle

IoC, SRP and composition - am I creating too many interfaces?


I'm writing an offline backup tool (in C# / .NET Core, if that matters) for multiple source code hosters, e.g. GitHub and Bitbucket.

There will be class for each hoster (GithubHoster, BitbucketHoster and so on) implementing the same interface.

I want the tool to be extensible, so it's easily possible to add more hosters just by creating some classes that are automatically picked up by IoC auto-registration.

For each hoster, the tool has to:

  • validate the settings for that hoster in the tool's config file
  • connect to the hoster's API and get a list of repository URLs
  • execute the proper source control tool to clone/pull all repositories to the local computer

This is obviously too much to put into one single class, so I used composition (or what I think composition means) to split it into subparts:

interface IHoster
{
    IConfigSourceValidator Validator { get; }
    IHosterApi Api { get; }
    IBackupMaker BackupMaker { get; }
}

interface IConfigSourceValidator
{
    Validate();
}

interface IHosterApi
{
    GetRepositoryList();
}

interface IBackupMaker
{
    Backup();
}

Problem: in order to inject the sub-classes into the IHoster implementation, I can't directly use the sub-interfaces shown above, because then the container wouldn't know which implementation to inject.

So I need to create some more empty marker interfaces especially for that purpose:

interface IGithubConfigSourceValidator : IConfigSourceValidator
{
}

interface IGithubHosterApi : IHosterApi
{
}

interface IGithubBackupMaker : IBackupMaker
{
}

...so I can do this:

class GithubHoster : IHoster
{
    public GithubHoster(IGithubConfigSourceValidator validator, IGithubHosterApi api, IGithubBackupMaker backupMaker)
    {
        this.Validator = validator;
        this.Api = api;
        this.BackupMaker = backupMaker;
    }

    public IConfigSourceValidator Validator { get; private set; }
    public IHosterApi Api { get; private set; }
    public IBackupMaker BackupMaker { get; private set; }
}

...and the container knows which implementations to use.

And of course I need to implement the sub-classes:

class GithubConfigSourceValidator : IGithubConfigSourceValidator
{
    public void Validate()
    {
        // do stuff
    }
}

// ...and the same for GithubHosterApi and GithubBackupMaker

This works so far, but somehow it feels wrong.

I have the "basic" interfaces:

IHoster
IConfigSourceValidator
IHosterApi
IBackupMaker

..and all the classes and interfaces for GitHub:

IGithubConfigSourceValidator
IGithubApi
IGithubBackupMaker
GithubHoster
GithubConfigSourceValidator
GithubApi
GithubBackupMaker

And each time I add a new hoster in the future, I have to create all this again:

IBitbucketConfigSourceValidator
IBitbucketApi
IBitbucketBackupMaker
BitbucketHoster
BitbucketConfigSourceValidator
BitbucketApi
BitbucketBackupMaker

Am I doing this right?

I know I need all the classes because I'm using composition instead of putting everything into one god class (and they're easier to test because each of them does only one thing).

But I don't like the additional interfaces I have to create for each IHoster implementation.

Maybe in the future it makes sense to split my hosters into more sub-classes, and then I'll have four or five of those interfaces per implementation.
...which means that after implementing support for a few additional hosters, I'll end up with 30 or more empty interfaces.


Additional info, requested by @NightOwl888:

My application supports backing up from multiple source code hosters, so it can use multiple IHoster implementations at runtime.

You can put username etc. for multiple hosters into a config file, with "hoster": "github", "hoster": "bitbucket" and so on.

So I need a factory which takes the strings github and bitbucket from the config file, and returns GithubHoster and BitbucketHoster instances.

And I want the IHoster implementations to provide their own string value, so I can easily auto-register them.

Because of that, the GithubHoster has an attribute with the string github:

[Hoster(Name = "github")]
internal class GithubHoster : IHoster
{
    // ...
}

And here's the factory:

internal class HosterFactory : Dictionary<string, Type>, IHosterFactory
{
    private readonly Container container;

    public HosterFactory(Container container)
    {
        this.container = container;
    }

    public void Register(Type type)
    {
        if (!typeof(IHoster).IsAssignableFrom(type))
        {
            throw new InvalidOperationException("...");
        }

        var attribute = type.GetTypeInfo().GetCustomAttribute<HosterAttribute>();
        this.container.Register(type);
        this.Add(attribute.Name, type);
    }

    public IHoster Create(string hosterName)
    {
        Type type;
        if (!this.TryGetValue(hosterName, out type))
        {
            throw new InvalidOperationException("...");
        }
        return (IHoster)this.container.GetInstance(type);
    }
}

Note: if you want to see the real code, my project is public on GitHub and the real factory is here

On startup, I get all IHoster implementations from Simple Injector and register each one in the factory:

var hosterFactory = new HosterFactory(container);
var hosters = container.GetTypesToRegister(typeof(IHoster), thisAssembly);
foreach (var hoster in hosters)
{
    hosterFactory.Register(hoster);
}

To give proper credit, the factory was created with a lot of help from Steven, the creator of Simple Injector.


Solution

  • Since for now you've not specified the IoC container, I would say that most of them should let you implement your own auto-registration conventions.

    See your hoster class:

    class GitHubHoster : IHoster
    {
        // Implemented members
    }
    

    I see that there's a variable part on the class identifier which is the provider. For example, GitHub, right? I expect that the rest of interfaces' identifiers should follow the same convention.

    Based on the provider prefix, you can assist your IoC container to create explicit dependencies.

    For example, Castle Windsor would do it as follows:

    Component.For<IHoster>()
             .ImplementedBy<GitHubHoster>()
             .DependsOn(Dependency.OnComponent(typeof(IHosterApi), $"{provider}HosterApi"))
    

    Now it's all about using some reflection to find out implementations to your interfaces and register them all based on the hint I've provided you above!