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

Question

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.

Answer 1

am I creating too many interfaces?

Yes, you are creating too many interfaces. You can simplify this greatly by making the interfaces generic, so all of the services are "keyed" for a specific entity. This is similar to what is commonly done for a repository pattern.

Interfaces

public interface IHoster<THoster>
{
    IConfigSourceValidator<THoster> Validator { get; }
    IHosterApi<THoster> Api { get; }
    IBackupMaker<THoster> BackupMaker { get; }
}

public interface IConfigSourceValidator<THoster>
{
    void Validate();
}

public interface IHosterApi<THoster>
{
    IList<string> GetRepositoryList();
}

public interface IBackupMaker<THoster>
{
    void Backup();
}

IHoster<THoster> Implementations

Then your IHoster<THoster> implementations would look like this:

public class GithubHoster : IHoster<GithubHoster>
{
    public GithubHoster(
        IConfigSourceValidator<GithubHoster> validator, 
        IHosterApi<GithubHoster> api, 
        IBackupMaker<GithubHoster> backupMaker)
    {
        if (validator == null)
            throw new ArgumentNullException("validator");
        if (api == null)
            throw new ArgumentNullException("api");
        if (backupMaker == null)
            throw new ArgumentNullException("backupMaker");

        this.Validator = validator;
        this.Api = api;
        this.BackupMaker = backupMaker;
    }

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

public class BitbucketHoster : IHoster<BitbucketHoster>
{
    public BitbucketHoster(
        IConfigSourceValidator<BitbucketHoster> validator,
        IHosterApi<BitbucketHoster> api,
        IBackupMaker<BitbucketHoster> backupMaker)
    {
        if (validator == null)
            throw new ArgumentNullException("validator");
        if (api == null)
            throw new ArgumentNullException("api");
        if (backupMaker == null)
            throw new ArgumentNullException("backupMaker");

        this.Validator = validator;
        this.Api = api;
        this.BackupMaker = backupMaker;
    }

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

Your other classes would also just need to be keyed to the GithubHoster or BitbucketHoster concrete service the same as above (even if the generic is not actually needed for the interface).

Simple Injector Configuration

With that done, now DI configuration is straightforward:

// Compose DI
var container = new Container();

IEnumerable<Assembly> assemblies = new[] { typeof(Program).Assembly };

container.Register(typeof(IHoster<>), assemblies);
container.Register(typeof(IConfigSourceValidator<>), assemblies);
container.Register(typeof(IHosterApi<>), assemblies);
container.Register(typeof(IBackupMaker<>), assemblies);


var github = container.GetInstance<IHoster<GithubHoster>>();
var bitbucket = container.GetInstance<IHoster<BitbucketHoster>>();

NOTE: If you want your application to be able to switch between IHoster implementations at runtime, consider passing a delegate factory to the constructor of the service that uses it, or use the strategy pattern.

Answer 2

You can use a Attribute to help you figure out which implementation are from where. This would avoid you having to create a lot of new interfaces.

Define this attribute

public class HosterAttribute : Attribute
{
    public Type Type { get; set; }

    public HosterAttribute(Type type)
    {
        Type = type;
    }
}

Create the other classes for the IGithubHoster and add the HosterAttribute like so

[HosterAttribute(typeof(IGithubHoster))]
class GithubBackupMaker : IBackupMaker
{
    public void Backup()
    {
        throw new NotImplementedException();
    }
}
[HosterAttribute(typeof(IGithubHoster))]
class GithubHosterApi : IHosterApi
{
    public IEnumerable<object> GetRepositoryList()
    {
        throw new NotImplementedException();
    }
}

[HosterAttribute(typeof(IGithubHoster))]
class GithubConfigSourceValidator : IConfigSourceValidator
{
    public void Validate()
    {
        throw new NotImplementedException();
    }
}

I haven't used the IoC you're using so you may need to modify the below method to suit your needs. Create a method that finds the relevant IHoster implementations for you. Here is one way.

public T GetHoster<T>() where T: IHoster
{
    var validator = Ioc.ResolveAll<IConfigSourceValidator>().Where(x => x.GetType().GetCustomAttribute<HosterAttribute>().Type == typeof(T)).Single();
    var hosterApi = Ioc.ResolveAll<IHosterApi>().Where(x => x.GetType().GetCustomAttribute<HosterAttribute>().Type == typeof(T)).Single();
    var backupMaker = Ioc.ResolveAll<IBackupMaker>().Where(x => x.GetType().GetCustomAttribute<HosterAttribute>().Type == typeof(T)).Single();

    var hoster = Ioc.Resolve<T>(validator, hosterApi, backupMaker);
    return hoster;
}

Now, in your programs code, all you have to do is

var gitHub = GetHoster<IGithubHoster>();
//Do some work

Hope this helps

Answer 3

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!

Answer 4

Okay, I just came up with another possible solution for my own question.

I'm posting it here for others to comment and upvote/downvote, because I'm not sure whether it's a good solution.

I'll get rid of the marker interfaces:

class GithubBackupMaker : IBackupMaker { ... }

class GithubHosterApi : IHosterApi { ... }

class GithubConfigSourceValidator : IConfigSourceValidator { ... }

...and I'll inject the Github... classes directly into the GithubHoster:

class GithubHoster : IHoster
{
    public GithubHoster(GithubConfigSourceValidator validator, GithubHosterApi api, GithubBackupMaker 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; }
}

I don't even need to register the sub-classes in Simple Injector, because it resolves classes even when they weren't registered before.

This means I can't test the GithubHoster and mock the subclasses, though...but that doesn't matter, because I don't test the GithubHoster at all, because it's just an empty shell that holds the subclasses.

---

I'm not sure whether it's an good solution, because all Dependency Injection examples on the internet always inject interfaces...you don't see a lot of examples where someone injects classes.

But I think in this special case the use of a class instead of an interface is justified.
Plus, after I finished writing this answer, I found someone who says it's a good solution for cases like this.

---

As an answer to @Matías Fidemraizer's comment:

I understand the arguments why you should use abstractions instead of implementations in general.

But my question was: What do I gain from using an abstraction in this special case?
In this context, I think I don't understand your answer:

First of all, the GithubHoster and all its subclasses do have interfaces:

class GithubHoster : IHoster
class GithubBackupMaker : IBackupMaker
class GithubHosterApi : IHosterApi
class GithubConfigSourceValidator : IConfigSourceValidator

I can still switch to another GithubHoster implementation without effort, because my factory returns an IHoster and all other places are just depending on IHoster.

So you can provide improvements without having to force anyone to use the newest dependency versions.

I'm writing an executable app, not a library.
So if I change the dependencies, I'm doing it because I want my app to use the newer version. So I wouldn't want any place in my app to not use the newest version of the dependencies.

So, my idea was to change this:

public GithubHoster(IGithubConfigSourceValidator validator, 
                    IGithubHosterApi api,
                    IGithubBackupMaker backupMaker) { }

...into this:

public GithubHoster(GithubConfigSourceValidator validator, 
                    GithubHosterApi api, 
                    GithubBackupMaker backupMaker) { }

No matter how I do it, I know that:

• those sub-classes will be used nowhere but in the GithubHoster
• the GithubHoster will always use those sub-classes and no other

So they're basically tightly coupled, I just split them into multiple classes because of SRP.

Any other means of expressing the fact that they belong to each other (marker interfaces /attributes / custom registration conventions that depend on all class names starting with the same letter) just seem like more ceremony to me, just for the sake of "thou shalt use abstractions everywhere" :-)

Maybe I'm really missing something, but for now I don't understand what it is.