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

Replacing Sigletons with mocks #615

Open
apobekiaris opened this Issue Sep 20, 2018 · 7 comments

Comments

2 participants
@apobekiaris

apobekiaris commented Sep 20, 2018

I am registering a collection of IExchange sigletons like

var container = ServiceContainer.Instance;
container.Options.AllowOverridingRegistrations = true;
var exchangeTypes = container.GetTypesToRegister(typeof(IExchange), typeof(IExchange).Assembly);
var exchangeRegistrations = exchangeTypes.Select(type =>
    Lifestyle.Singleton.CreateRegistration(type, container)).ToArray();
container.Collection.Register<IExchange>(exchangeRegistrations);

now I try to replace those instances with their mocks so I modify my code

var exchangeRegistrations = exchangeTypes.Select(type =>
    Lifestyle.Singleton.CreateRegistration(() => 
        (IExchange)((Mock) typeof(Mock<>).MakeGenericType(type).CreateInstance()).Object,
    container)).ToArray();
container.Collection.Register<IExchange>(exchangeRegistrations);

what is the appropriate call to get an instance of an IExchange? I only found the GetAllInstances method which works fine but does not feel like the best approach.

container.GetAllInstances<IExchange>().OfType<ConcreteExchange>().First()
@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Sep 20, 2018

What's the point of replacing all IExchange implementations with the same number of mock implementations? This seems like an unobvious thing to do.

@apobekiaris

This comment has been minimized.

apobekiaris commented Sep 20, 2018

but I need to mock certain methods (web calls) of IExchange so I can test them, how else this can be done?

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Sep 20, 2018

When dealing with a sequence of things, such as your IEnumerable<IExchange>, the consumer would typically treat all instances of that abstraction equal, for instance by iterating over them all and calling an IExchange method on all of them. So from perspective of the consumer, it shouldn't matter whether it has 1 or many. As a matter of fact, there is a pattern that allows the consumer to stay oblivious to the fact that it has to deal with a multiplicity. This is the Composite pattern.

In your case, the Composite would be an implementation of IExchange, while at the same time wrapping a sequence of IExchange instance. In other words, the Composte would depend on IEnumerable<IExchange>. This allows the consumer to simply take a dependency on IExchange.

Not only would taking a dependency on IExchange rather than IEnumerable<IExchange> simplify the consumer, it can also simplify testing, because testing the consumer would only mean testing the interaction between the consumer and IExchange, rather than the consumer and an IEnumerable<IExchange>.

It would also allow simplifying integration tests, which you seem to be doing, considering the fact that you are using your DI Container in your test. (A unit test never uses a DI Container.) So in case IExchange implementations need to be mocked, the use of the Composite allows you to simply replace that Composite instead of having to replace all implementations.

@apobekiaris

This comment has been minimized.

apobekiaris commented Sep 20, 2018

Thanks for the detailed answer. A different design would help a lot I totally agree.

When dealing with a sequence of things, such as your IEnumerable<IExchange>, the consumer would typically treat all instances of that abstraction equal

For my system though the IExchange is a dependency without a way to inject it and the code uses the container to instantiate it. My guess is that I have to inject it to solve my problem. However it also looks like that the

container.GetAllInstances<IExchange>().OfType<ConcreteExchange>().First()

is actually a Composite implementation which depends on IEnumerable<IExchange>, IExchange so on a second thought it looks like I am safe to keep my current

public static IExchange GetExchange(this Container container,Type exchangeType){
    return container.Options.AllowOverridingRegistrations
        ? container.GetAllInstances<IExchange>().First(exchangeType.IsInstanceOfType)
        : (IExchange) container.GetInstance(exchangeType);
}
@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Sep 21, 2018

If you wish to use a Composite, the trick with Simple Injector is to do the following:

// Register the composite as one-to-one mapping
container.Register<IExchange, CompositeExchange>(Lifestyle.Singleton);

// Register all concrete composites as collection while preventing the composite from being wrapped
var exchangeTypes = container.GetTypesToRegister(typeof(IExchange), typeof(IExchange).Assembly,
    new TypesToRegisterOptions { IncludeComposites = false });
var exchangeRegistrations = exchangeTypes.Select(type =>
    Lifestyle.Singleton.CreateRegistration(type, container)).ToArray();
container.Collection.Register<IExchange>(exchangeRegistrations);

Alternatively, if you're not interested in those exchanges being registered as singleton, you can simplify the registration to the following:

container.Register<IExchange, CompositeExchange>(Lifestyle.Singleton);

// Collection.Register will automatically filter out any composites.
container.Collection.Register<IExchange>(typeof(IExchange).Assembly);

What both registrations achieve is that they register the Composite as one-to-one mapping, while having all other (non composite, non decorator) implementations be registered as part of the collection. This ensure that when a single IExchange is resolved, it returns a CompositeExchange that wraps a collection of all other exchanges.

@apobekiaris

This comment has been minimized.

apobekiaris commented Sep 21, 2018

could you also post how the CompositeExchange might looked like

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Sep 21, 2018

public class CompositeExchange : IExchange
{
    private readonly IEnumerable<IExchange> exchanges;
    public CompositeExchange(IEnumerable<IExchange> exchanges) => this.exchanges = exchanges;

    public void ExchangeMethod() {
        foreach (var exchange in this.exchanges) exchange.ExchangeMethod();
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment