simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 154 forks source link

Structure and layout of project with DI framework #411

Closed christophedemey closed 7 years ago

christophedemey commented 7 years ago

Hi

I would like to know how to layout my project and where to put what when using a DI framework like Simple Injector.

  1. I now have my ComposeRoot at the entry point of my application, is this correct ?
  2. Where do i put my factory implementations ? If i assume point 1 is correct i would require a concrete implementation at my entry point level which seems weird cause i think it would belong more in the bussineslogic part of my app Or do i put all the DI and factories in a seperate project.
  3. Any specific project where i should put my interfaces ? For now i just put them under my models project.

Here is my small test project in which i am trying to learn these things with Simple Injector. Repository : https://github.com/christophedemey/DI-SimpleInjector/

Any help is greatly appreciated.

Thanks

dotnetjunkie commented 7 years ago

I now have my ComposeRoot at the entry point of my application, is this correct ?

That is correct. The Composition Root is:

a (preferably) unique location in an application where modules are composed together.

Where do i put my factory implementations ? If i assume point 1 is correct i would require a concrete implementation at my entry point level which seems weird cause i think it would belong more in the bussineslogic part of my app Or do i put all the DI and factories in a seperate project.

It depends. In general, you should hardly have any factories at all, as described here. But if you need them, it depends on what that factory does.

  1. The rule is that only the Composition Root should have access to the DI Container, so if your factory requires resolving services from the Container, that factory implementation should be placed inside the Composition Root.
  2. The same holds if your factory manually creates application components (using Pure DI); the Composition Root is where we wire our object graphs.
  3. If on the other hand, the factory does 'other stuff' (whatever that may be), it's fine to have them in code, as long as they don't do things described in 1 or 2.

Any specific project where i should put my interfaces ? For now i just put them under my models project.

There are many ways to do this. The most important part is that you program against interfaces. Where you place your interfaces can be very important for some projects, while in other cases it doesn't really matter. Putting abstractions and implementations in different projects is important when:

  1. The implementation requires a dependency to some library, technology or framework, while the interface's assembly should not have that dependency. Think about having an IUserContext abstraction that your business layer requires, while the implementation is specific to ASP.NET. You may want to be able to run that business logic outside the context of an ASP.NET application.
  2. When parts of the application should not always be deployed on all machine. Think about a rich client (WPF, Win Forms) that communicates with a Business Layer that runs on a web service. The client might need to use the same abstractions or send messages of a type that is known to the Business Layer, while we want to prevent that Business Layer to be deployed on the client machines. Perhaps the BL contains company secrets that we don't want to share. Or perhaps that BL depends on some library that is unavailable on the client (think about Entity Framework when we create a Mobile .NET app).
christophedemey commented 7 years ago

Hi dotnetjunkie

How would i proceed implementing an IFactory<T>. I have :

container.Register<IFactory<IStationController>, FactoryController<IStationController>>(
    Lifestyle.Singleton);

And then the implementation :

public class FactoryController<T> : IFactory<T>
{
    private Container container = null;

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

    public T CreateInstance()
    {
        // what to do here?
    }
}

Resolve test:

var opo = container.GetInstance<IFactory<IStationController>>();

I don't really know what to type in the CreateInstance function. I tried Container.GetInstance(); but that yielded the error :

The type 'T' must be a reference type in order to use it as parameter 'TService' in the generic type or method Container.GetInstance<TService>().

Thanks.

dotnetjunkie commented 7 years ago

The compile error you get is caused by the generic type constraint on the Container.GetInstance<T>() method. It is specified as follows:

public TService GetInstance<TService>() where TService : class { ... }

So note the where TService : class type constraint. This means that your method or class should have this type constraint as well.

This is how your factory should look like:

public sealed class SimpleInjectorFactory<T> : IFactory<T> where T : class {
    private readonly Container container;
    public FactoryController(Container container) { this.container = container; }
    public T CreateInstance() => this.container.GetInstance<T>();
}

And this is how to register it:

container.RegisterSingleton<IFactory<IStationController>>(
    new SimpleInjectorFactor<IStationController>(container));

But you can also register it as open-generic type, which is especially useful when you have many closed versions of IFactory<T> that you wish to inject:

container.Register(typeof(IFactory<>), typeof(SimpleInjectorFactory<>), Lifestyle.Singleton);

I would like to repeat my previous warning about having factories: They should hardly ever exist at all.

christophedemey commented 7 years ago

Hi dotnetjunkie

Thank you for the answer, this did resolve the problem.

Regarding your warning : If factories shouldn't exist then how would i resolve multiple object graphs at runtime ? For example, the end-user has an Add Station button in the UI and clicks it 3x, how would i resolve 3 new IStationController's without using a delegate or factory ? Or is it in this case permitted to have them ?

Thanks.

dotnetjunkie commented 7 years ago

If factories shouldn't exist then how would i resolve multiple object graphs at runtime ?

Did you read the article? It doesn't say that you aren't allowed to resolve stuff from the container; what it says is that there shouldn't be an application abstraction that returns another application abstraction from its methods. Instead, the used abstraction should hide that the fact that this other abstraction exists.

I know too little about your specific example to say anything about it, but what I noticed in your original Stackoverflow question is that your IStationController is mutable, gets initialized with runtime data and depends on Temporal Coupling.

As described here, your application components should not require runtime data during initialization. That article also describes how to come around that.

I hope this helps.

christophedemey commented 7 years ago

Hi dotnetjunkie

Yes i read the article and understand, thanks for your patience. In my test-repository i have 4 Implementations of ITool.

public interface ITool
 {
    string Name { get; }
    void Enable();
    void Disable();
}

I register them as seen in the SimpleInjector manual.

var toolAssembly = typeof(AmazingTool).Assembly;
Assembly[] assemblies = new Assembly[] { toolAssembly };
container.RegisterCollection(typeof(ITool), new List<Assembly> { toolAssembly });

Which means i can get them by adding this in my constructor :

public ToolController(IEnumerable<ITool> tools)

And then i can list them out :

Console.WriteLine("Available tool types :");
foreach (ITool tool in tools)
{
    Console.WriteLine($"- {tool.Name}");
}
Console.WriteLine();

Now say i want an implementation which contains tool.Name = "AmazingTool" by clicking on a button. At this point i have :

container.RegisterSingleton<Func<string, ITool>>(
    (toolType) => container.GetAllInstances<ITool>()
        .Where(row => row.Name == toolType)
        .FirstOrDefault());

Which seems a bit overkill since im instantiating all the classes just to get that 1 out of the collection. How would i implement only resolving 1?

Thanks so far for the help so far, it is greatly appreciated!

dotnetjunkie commented 7 years ago

Unless looping over the collection causes the creation of hundreds of objects, doing this isn't normally a problem. Resolving instances in Simple Injector is almost free.

The documentation however describes how you can resolve instances by key which might be what you are looking for (but again, do measure performance first).

There's a catch though considering key based registration. As with any DI container, resolving instances based on a name, requires names to be constant. This might still be in your case, but your design tells us otherwise. Name is an instance member on the interface, which means that a tool could theoretically change its name on the fly.

Even if your tool instances don't do this, your design communicates differently. In case the tool name is static for its type, not for its instance, you might want to model it accordingly. A common solution is to use attributes instead, since attributes are coupled to a type, not an instance. Or you can completely decouple the name from the tool type, and instead only let the Composition Root know about the relationship between tools and their names.

christophedemey commented 7 years ago

Ok, got it.

Could you show me an example of using Attributes to name my implementations instead of using the Name property ? And how would i obtain that Attribute value to print out the available implementations.

Thanks.

dotnetjunkie commented 7 years ago

Could you show me an example of using Attributes to name my implementations instead of using the Name property ?

[Name("Awesome")]
public class AwesomeTool : ITool
{
    public void Enable() => ...
    public void Disable() => ...
}

public sealed class NameAttribute : Attribute 
{
    public readonly string Name;
    public NameAttribute(string name) { Name = name; }
}

string name = typeof(AwesomeTool).GetCustomAttribute<NameAttribute>().Name;
christophedemey commented 7 years ago

Hi dotnetjunkie

I have removed the Name property from the interface and added the attribute to my 4 test implementations.

I am now listing them like this:

public void PrintToolTypes()
{
    Console.WriteLine("Available tool types :");
    foreach (ITool tool in tools)
    {
        Console.WriteLine($"- {tool.GetType().GetCustomAttribute<NameAttribute>().Name}");
    }
    Console.WriteLine();
}

And resolving them like this :

public ITool CreateToolOfType(string toolType)
{
    return toolFactory.Invoke(toolType);
}

Delegate factory :

//Register tool factory.
container.RegisterSingleton<Func<string, ITool>>(toolType => 
    container.GetAllInstances<ITool>()
    .Where(row => row.GetType().GetCustomAttribute<ToolNameAttribute>().Name == toolType)
    .FirstOrDefault());

I understand this makes my design scream the Name can not be changed. But it adds some more complexity and i wonder when do you decide if it's worth it. What do you think of the way i changed my code, any suggestions ?

Thanks you for your patience and time in this "issue" : -)

Btw : I am still updating the repository linked in the first post if you would want to take a look.

christophedemey commented 7 years ago

Hi dotnetjunkie

If i would want to move my factories and DI container into it's seperate project, what would i call this ? The reason i ask is that it bothers me a little having a concrete implementation of the IFactory in my ConsoleApp6 project. It doesnt feel right, hehehe feelings.

Thanks.

christophedemey commented 7 years ago

Hi dotnetjunkie

I was playing arround with the system but found something strange. When my ToolController implementation is instantiated it gets the IEnumerable<ITool> injected. This is done by this register :

//Register all tools which reside in the same assembly as the concrete AmazingTool implementation.
var toolAssembly = typeof(AmazingTool).Assembly;
Assembly[] assemblies = new Assembly[] { toolAssembly };
container.RegisterCollection(typeof(ITool), new List<Assembly> { toolAssembly });

The tool controller implementation :

private IEnumerable<ITool> tools = null;
private IToolFactory toolFactory = null;

public ToolController(IEnumerable<ITool> tools, IToolFactory toolFactory)
{
    this.tools = tools;
    this.toolFactory = toolFactory;
}

Now when i IEnumerate them it will create a new instance each time.

public void PrintToolTypes()
{
    Console.WriteLine("Available tool types :");
    foreach (ITool tool in tools)
    {
        Console.WriteLine($"- {tool.GetType().GetCustomAttribute<ToolNameAttribute>().Name}");
    }
    Console.WriteLine();
}

I found this by putting a breakpoint on the constructor of my AmazingTool implementation.

Please review this screenshot:

Screenshot

dotnetjunkie commented 7 years ago

Now when i IEnumerate them it will create a new instance each time.

Yes, that's intentional. With Simple Injector IEnumerable<T> is a stream:

Simple Injector preserves the lifestyle of instances that are returned from an injected IEnumerable<T>, ICollection<T>, IList<T>, IReadOnlyCollection<T> and IReadOnlyList<T> instance. In reality you should not see the the injected IEnumerable<T> as a collection of instances; you should consider it a stream of instances. Simple Injector will always inject a reference to the same stream (the IEnumerable<T> or ICollection<T> itself is a singleton) and each time you iterate the IEnumerable<T>, for each individual component, the container is asked to resolve the instance based on the lifestyle of that component.

If you don't want this to happen, you can inject an array instead.

dotnetjunkie commented 7 years ago

If i would want to move my factories and DI container into it's seperate project, what would i call this ?

Typically a bad idea. The place where you configure your your container is called the Composition Root and this Composition Root should be located as close as possible to the application's entry point.

dotnetjunkie commented 7 years ago

But it adds some more complexity and i wonder when do you decide if it's worth it.

What I understand from your code examples is that you actually need the name outside the Composition Root as well. You not only use it to select the proper tool, but you use it to display to the user as well. Having to use GetCustomAttribute<T> as part of your application logic is a bad idea, because this would break once you wrap your tool instances with a decorator. So, from that perspective, the use of a Name property makes more sense, but that does mean you will have to reflect over the complete collection at runtime to find the proper instance.

There are multiple ways however to speed that up -if required. Let me know when this is required.

christophedemey commented 7 years ago

Hi dotnetjunkie,

Thank you for taking the time to answer my (noob) questions. -Q1 : IEnumerable<T> calls constructor of instance inside. Ok, please correct me if im wrong. ToolController is instantiated , the IENumerable<ITool> is injected, no instances of my concrete implementations of ITool are created yet. When i enumerate them for the first time, they are resolved during enumeration. When i enumerate them a second time (same collection) they are streamed, not calling the constructor.

I found this because i had Console.Writeline("Tool Created"); in the all tools constructors. And then found this output multiple times.

Q2- : Composition root : OK Q3- : Back to the Name property :) Ok, how can i speed this up in a generic way ? I had my concerns about this already but you said resolving instances was pretty much free so i didn't continue on that topic.

dotnetjunkie commented 7 years ago

Hi Christophe,

-Q1 : IEnumerable<T> calls constructor of instance inside.

You can best compare the behavior of Simple Injector around IEnumerable<T> with a Entity Framework LINQ query:

IEnumerable<Order> orders = from o in db.Orders where o.Price > 100 select order;

Every time you iterate the collection, the database is queried and results are 'streamed' from the database. They are loaded one by one while iterating the list.

This is the same behavior as you see with Simple Injector. The IEnumerable<T> is just a wrapper, a factory. Once you start iterating it, for each registration in the list, the enumeable will ask Simple Injector for an instance. Simple Injector will resolve that instance according to its lifestyle.

This means that in case the lifestyle is Transient a new instance will be created every time. However in case it is Scoped, it might return the same instance. And in case it's Singleton, you will always get the same instance.

Take a look at this registration for instance:

var container = new Container();
container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

container.Register<Tool1>(Lifestyle.Singleton);
container.Register<Tool2>(Lifestyle.Scoped);
container.Register<Tool3>(Lifestyle.Transient);
container.RegisterCollection<ITool>(new[] { typeof(Tool1), typeof(Tool2), typeof(Tool3) });

var tools = container.GetInstance<IEnumerable<ITool>>();

using (AsyncScopedLifestyle.BeginScope(container))
{
    tools.ToArray();
    tools.ToArray();
}

In the previous example the collection is iterated twice (using ToArray()). However since two registrations are Scoped and Singleton, they will only be created once, while the third one is created twice.

The reason Simple IEnumerables are streams in Simple Injector is because this simplifies certain scenarios. For instance, it allows instances to be created lazily, which increases performance on large collections, especially when not all instances are required (for instance because of filtering). For instance:

// Imagine hundreds of tools
container.RegisterCollection<ITool>(new[] { typeof(ITool).Assembly });

var tools = container.GetInstance<IEnumerable<ITool>>();

var tool = tools.First(tool => tool.Name == name);

The First() call will iterate the collection until the predicate is matched. This means that no more instances are created after the correct tool is found. Imagine hundreds of tools while the name is the first one. That will only result in the creation of 1 tool.

Streams also prevent lifestyle mismatches, because it is quite normal to have elements within the collection with mixed lifestyles. Without having streams, you are almost always forced to let the consumer have a transient lifestyle.

When i enumerate them a second time (same collection) they are streamed, not calling the constructor.

This depends on their lifestyle. In case they are transient, new instances are created.

how can i speed this up in a generic way ?

That's a difficult question to answer. The streams can help you. Changing lifestye might help. It might even help to make tools generic, as in ITool<T>, but that depends completely on your needs. But for inspiration, take a look at this blog post. It talks about designs using generic typing, which can be really powerful. I don't know however if this suits your needs.

christophedemey commented 7 years ago

Hi dotnetjunkie

Pfew, getting there, but slowly :-) When i am using the Transient lifestyle which is the default one for the RegisterCollection method. I see that when the IEnumerable<ITool> gets injected into my IToolController that all of my implementations are constructed without me iterating them.

ToolController created - Gets injected IENumerable<ITool Shoot Yourself In The Face Tool tool created.
Not So Amazing Tool tool created.
Habanero Tool tool created.
Amazing Tool tool created.
Woop Tool tool created.

And then they are created again when i Iterate them.

Listing Available tool types :
Shoot Yourself In The Face Tool tool created.
Shoot Yourself In The Face Tool
Not So Amazing Tool tool created.
Not So Amazing Tool
Habanero Tool tool created.
Habanero Tool
Amazing Tool tool created.
Amazing Tool
Woop Tool tool created.
Woop Tool

And it is streamed when i perform a lambda until it reaches the implementation that matches my criteria, i can see this here as "Woop Tool" is not outputted, exactly as you explained (db example).

Requesting instance of ITool with name Amazing Tool
Shoot Yourself In The Face Tool tool created.
Not So Amazing Tool tool created.
Habanero Tool tool created.
Amazing Tool tool created.
Instance of Amazing Tool resolved.

So to prevent my instances from being created again in the foreach loop i figured i would inject an array of ITool, this way the instances would be resolved 1 time as they are not streamed like an IENumerable as you suggested a few posts upwards. But then i get greeted with this error : -[Lifestyle Mismatch] ToolController (Singleton) depends on ITool[] (Transient).

It injects fine, i can assign it to my private scoped field, but when i try to foreach the collection i get that error.

I also took a look at your scoped example which makes allot of sense until i try to apply it to my scenario then i have to pick out of these lifestyles which dont seem to match my requirements. Please correct me if im wrong.

I have also taken a look at your article, it's pretty crazy :-) I will try implementing this into a small sample app when i get this one to work properly.

Again thanks for your time and patience.

dotnetjunkie commented 7 years ago

I see that when the IEnumerable gets injected into my IToolController that all of my implementations are constructed without me iterating them.

Are you calling container.Verify() at the end of the registration process?

But then i get greeted with this error :

You can find more information about this type of Diagnostic warning here. Please read carefully before continue.

Reason that you get this error with array, but not with IEnumerable<T> is because before injecting the collection, the array is copied. An array is not a stream and iteration will not create them over and over again. However, the consumer is a singleton, which is why this breaks.

This is exactly the reason that enumerables are streams in Simple Injector. It allows you to safely inject them into a singleton component. If this is not what you want, either make your tools singleton, or your consumer transient.

i have to pick out of these lifestyles which dont seem to match my requirements. Please correct me if im wrong.

I don't know your requirements, so I don't know whether you're wrong.

I have also taken a look at your article, it's pretty crazy :-)

Not crazy at all, but it is a programming style that is quite unfamiliar to many developers and it takes time to get used to. It's not suited for every application and every solution, but you'll find variations of this pattern useful in many scenarios. For instance:

Wrapping business operations around ICommandHandler<T> becomes very useful when applications are not CRUDy and contain dozens of use cases. It's also very beneficial in Win Forms and WPF applications where Forms and Page and Window classes most often have a very long lifetime, while the business logic that connects to the database must use database objects with a much shorter lifestyle.

The use of IQueryHandler<TQuery, TResult> is very useful even for CRUD applications, because it simplifies applying cross-cutting concerns such as security and caching and prevents the consuming code with an uniform abstraction for returning data from a data source.

I will try implementing this into a small sample app when i get this one to work properly.

Please do. These patterns take time to master. It's important to do this in throw-away sample applications.

christophedemey commented 7 years ago

Hi dotnetjunkie

Are you calling container.Verify() at the end of the registration process?

Yes, if you want you can take a look at this code at https://github.com/christophedemey/DI-SimpleInjector

I don't know your requirements, so I don't know whether you're wrong.

I am trying to achieve a simple thing, i want to inject a List's in my IToolController constructor so i can list them out when requested and this without recreating all the objects all over again while iterating them, for example a UI will ask for the tool names and will populate a grid or listview. When the user clicks this listview and then the button create, it will create a new ITool instance by name.

I was thinking about this yesterday and thought maybe i can resolve this with 2 delegate registrations. 1 that gives me the list and 1 that gives me the instance by name. What do you think would be the proper way to do this ?

Thanks.

christophedemey commented 7 years ago

Hi dotnetjunkie

Ok, that works but i dont know if it's another bad design :P

The IToolFactory

public interface IToolFactory
    {
        List<ITool> CreateAllTools();
        ITool CreateInstance(string name);
    }

The implementation, i noticed here that if i dont return an IList<> & .ToList() i get the same behavior as before, just like when using a database.

public IEnumerable<ITool> CreateAllTools()
        {
            return container.GetAllInstances<ITool>().ToList();
        }

public ITool CreateInstance(string name)
        {
            return container.GetAllInstances<ITool>().
               Where(row => row.Name == name).
               FirstOrDefault();
        }

The injection

public ToolController(IToolFactory toolFactory)
        {
            this.toolFactory = toolFactory;
            this.tools = toolFactory.CreateAllTools();
        }

But now i can see the tools are created 1 time when injected, not while iterating. And when requesting a tool by name i can see the same performance benefits since the factory will request the data from the container, resolving until it meets the requirements of the query.

Thanks.

dotnetjunkie commented 7 years ago

Are you calling container.Verify() at the end of the registration process? Yes, if you want you can take a look at this code at

A call to verify will test the creation of all your registered components. I bet that's the reason why your tools are created.

i want to inject a List's in my IToolController constructor so i can list them out when requested and this without recreating all the objects all over again while iterating them, for example a UI will ask for the tool names and will populate a grid or listview. When the user clicks this listview and then the button create, it will create a new ITool instance by name.

Well, you will have to make up your mind. You don't want to recreate the tools, but you are registering them as transient, while injecting them in a class with a singleton lifestyle. If you inject them as array, it will cause a lifetime mismatch, because you stated that you always want a new one (transient).

If you don't want to recreate tools, register them as singleton:

Assembly[] assemblies = new Assembly[] { typeof(AmazingTool).Assembly };
var toolTypes = container.GetTypesToRegister(typeof(ITool), assemblies);
var registrations = 
    toolType.Select(type =>Lifestyle.Singleton.CreateRegistration(type, container)).ToArray();

container.RegisterCollection(typeof(ITool), registrations);

You might think Simple Injector is annoying, and it can be :), but it is created to protect you.

maybe i can resolve this with 2 delegate registrations. 1 that gives me the list and 1 that gives me the instance by name.

You can do something like this:

Assembly[] assemblies = new Assembly[] { typeof(AmazingTool).Assembly };
var toolTypes = container.GetTypesToRegister(typeof(ITool), assemblies);

container.RegisterCollection(typeof(ITool), toolTypes);

var producers = toolType
    .Select(type => Lifestyle.Transient.CreateProducer<ITool>(type, container))
    .ToArray();

var toolProducers = new Lazy<Dictionary<string, InstanceProducer<ITool>>>(
    () => producers.ToDictionary(p => p.GetInstance().Name));

container.RegisterSingleton<Func<string, ITool>>(
    tooltype => toolProducers.Value[toolType].GetInstance());

Well, a delegate acts like a factory, and an enumerable already is a factory, so you need one delegate and the enumerable. Didn;t you already solve this delegate problem?:

container.RegisterSingleton<Func<string, ITool>>(
    (toolType) => container.GetAllInstances<ITool>()
        .Where(row => row.Name == toolType)
        .FirstOrDefault());
dotnetjunkie commented 7 years ago

But now i can see the tools are created 1 time when injected, not while iterating.

You should never use your dependencies inside the constructor. There is a great article by Mark Seemann that describes why you shouldn't. I think the first edition of his book Dependency Injection in .NET talks about this as well, and the upcoming second edtion sure will.

christophedemey commented 7 years ago

Hi dotnetjunkie

Heehaw i was already afraid that the logic in the constructor was a nono :) I will take it out and call the toolFactory.CreateAllTools(); in the PrintTools() function. I will also test your producer example hopefully tomorrow and keep you posted.

Thanks.

christophedemey commented 7 years ago

Hi dotnetjunkie

Thanks for the example, i also got rid of my generic factory and replaced it with a delegate.

//Generic factory.
container.RegisterSingleton<Func<Type, object>>((type) => container.GetInstance(type));

Then using it like this after injected into the controller : var stationController = stationControllerFactory.Invoke(typeof(IStationController));

Is this OK or another design smell ? :D

Thanks.

dotnetjunkie commented 7 years ago

I would strongly advise against registing a Func<Type, object> delegate. It is an implementation of the Service Locator anti-pattern. Please take a good look at that linked article and understand why using a Service Locator is a really bad idea.

christophedemey commented 7 years ago

Hi dotnetjunkie

I understand the article and it makes perfect sense, i have reverted back to the GenericFactory implementation that at least shows the consumer of my DLL what is to be expected.

How would i add additional parameters to the constructor which can not be resolved by the container. For example if i would like to provide a value for string additionalValue :

private ISomeInterface someInterface = null;
private string additionalValue = string.Empty;

public ExampleClass(ISomeInterface someInterface, string additionalValue)
{
  this.someInterface = someInterface;
  this.additionalValue = additionalValue;
}

Or should this not be in the constructor and required to be supplied to the function which is consuming it ?

Thanks!

dotnetjunkie commented 7 years ago

How would i add additional parameters to the constructor which can not be resolved by the container.

It depends on what kind of value it is. From a DI perspective, we divide them in two catagories:

  1. Values that are constants; are determined once at application startup, and don't change afterwards. These are typically configuration values.
  2. Values that present some runtime data, that might change over time. This is typically contextual information, such as information about the current user, but can be some volatile configuration value as well, the value might be read over and over again from the database.

When it comes to the second category, the advice is simple: don't inject them into your components, as explained here.

With the first category, there are multiple ways to handle them. Often you'll see that classes that require these configuration values are leaf components with no dependencies of them own. That makes registering them quite easy:

container.Register<IExampleClass>(() => new ExampleClass("someValue"));

When such class contains dependencies of its own, things get a bit more complicated. When the dependency itself is a simple singleton, we can do something as follows:

ISomeInterface impl = new SomeImplementation();

container.RegisterSingleton(impl);
container.Register<IExampleClass>(() => new ExampleClass(impl, "someValue"));

This practically means that we don't use the container's auto-wiring capabilities for the creation of ExampleClass; we call its constructor by hand.

When the class gets more dependencies or the dependency isn't a simple precreated singleton, the use of auto-wiring will get more beneficial; not using auto-witing also means loosing the containers capability to verify the application's object graphs.

Although you could do the following:

container.Register<IExampleClass>(() => new ExampleClass(
    container.GetInstance<ISomeInterface>(),
    "someValue"));

This is not adviced, since this completely blinds the ability for Simple Injector to warn you if you made a configuration mistake, such as a Lifestyle Mismatch.

Although there are ways to override Simple Injector's ability to wire primitive dependencies, typically by defining a custom IDependencyInjectionBehavior, a more convenient way is to define a custom configuration object for the component. For instance:

public class ExampleClassConfiguration
{
    public readonly string AdditionalValue;

    public ExampleClassConfiguration(string additionalValue) {
        // TODO: Verify configuration values here
        this.AdditionalValue = additionalValue;
    }
}

Your application component can instead depend on the configuration class:

// Also note 2 things about these fields: 1. they are 'readonly' and 2. they are not initialized.
// readonly adds compiler support and initialization is already done by the CLR; its redundant.
private readonly ISomeInterface someInterface;
private readonly ExampleClassConfiguration configuration;

public ExampleClass(ISomeInterface someInterface, ExampleClassConfiguration configuration)
{
    this.someInterface = someInterface;
    this.configuration = configuration;
}

Registration now becomes trivial:

container.RegisterSingleton(new ExampleClassConfiguration("some value"));
container.Register<IExampleClass, ExampleClass>();
christophedemey commented 7 years ago

Hi dotnetjunkie

Trying to convert my sample application which uses UnitOfWork & Repository pattern but cant seem to quite figure out how to pass the DbContext reference from my UnitOfWork to my Repository.

My UnitOfWork implementation gets a DbContext and IRepository, the exact same DbContext should be passed into the Repository..

   public class SqlUnitOfWork : IUnitOfWork
    {
        private DbContext context = null;

        public SqlUnitOfWork(DbContext dbContext, IRepository<tblChatroom> chatrooms)
        {
            context = dbContext;
            Chatrooms = chatrooms;
        }

My Repository implementation gets a DbContext.

  public class SqlRepository<T> : IRepository<T> where T : class
    {
        private DbContext _entities = null;

        public SqlRepository(DbContext context)
        {
            _entities = context;
        }

How can i configure SimpleInjector that each time when i resolve the UnitOfWork instance that it needs to create 1 DbContext and pass it on to all of its leaf injections (Repository constructor dbContext) ?

Thanks.

dotnetjunkie commented 7 years ago

How can i configure SimpleInjector that each time when i resolve the UnitOfWork instance that it needs to create 1 DbContext and pass it on to all of its leaf injections (Repository constructor dbContext) ?

Using the scoped lifestyle:

container.Register<DbContext>(() => new MyAppDbContext("constr"), Lifestyle.Scoped);

For more information, read this.

christophedemey commented 7 years ago

Hi dotnetjunkie

Ive read the docs but am confused again. Now i need to wrap my code which resolves and uses implementation with something like this : using (ThreadScopedLifestyle.BeginScope(container)) This also means i need a reference to SimpleInjector and my container inside my bussineslogic code, or is there a way around this ? Also the scopes dont seem to match my needs at first glance. I would like a new instance of my unitofwork and repository each time i request it, and 1 dbContext which gets passed into unitofwork and the repository. The lifestyles Thread,Async,Web,Wcf dont seem applicable to this scenario. I also created another repo for this one : https://github.com/christophedemey/DI-SimpleInjector-UnitOfWork-Repository-Pattern

Thanks for your time.

dotnetjunkie commented 7 years ago

Ive read the docs but am confused again.

I don't think you are. I think you understood quite well that dependencies between application code and the container should be prevented like the plague; that's why you ask this question.

This also means i need a reference to SimpleInjector and my container inside my bussines logic code, or is there a way around this?

As you might imagine, there should be. However, whether you're able to do this depends on the quality of your design. So this is actually a good benchmark for your application design.

A great application design should make it easy to apply cross-cutting concerns, such as something like scoping in a way that doesn't affect your application code. This typically means that you will have to have generic abstractions in place.

With such design, you will be able to create 1 decorator that applies scoping as follows:

public class ThreadScopedLifestyleCommandHandlerDecorator<T> : ICommandHandler<T> {
    private readonly Container container;
    private readonly Func<ICommandHandler<T>> decorateeFactory;

    public ThreadScopedLifestyleCommandHandlerDecorator(
        Container container, Func<ICommandHandler<T>> decorateeFactory) {
        this.container = container;
        this.decorateeFactory = decorateeFactory;
    }

    public void Handle(T command) {
        using (ThreadScopedLifestyle.BeginScope(this.container)) {
            var decoratee = this.decorateeFactory();
            decoratee.Handle(command);
        }
    }
}

When wrapped around an ICommandHandler<T> implementation, this decorator will start a new scope and will ask the container to build an object graph for the decorated registration. After that registration is created, it will be used.

Simple Injector will natively understand how to work with Func<T> dependencies in decorators, when that T represents the decorated type. This means that by injecting an Func<ICommandHandler<T>> instead of a ICommandHandler<T>, we can postpone the creation of everything below the decorator, which means we can postpone it untill we wrapped it inside a scope. This is a really powerful concept.

Obviously, our decorator will have a very strong dependency on the used DI Container (Simple Injector in your case), but this shouldn't be a problem, because we can define this decorator as part of the Composition Root. The rest of your application code, that implements this (or another generic interface, such as an IQueryHandler<T,R> or IValidator<T>, or perhaps IRepository<T>) doesn't need to have any clue about the existance of a DI Container; and this is how it's supposed to be.

If however, your application contains many one-to-one interfaces (which means you violate the RAP), you'll find yourself having to create an enourmous amount of decorators to get the job done. This is a main reason for developers to revert to code weaving tools like PostSharp to interweave cross-cutting concerns in a post-compile step to the assembly, or dynamic proxy tools like Castle Dynamic Proxy that allow to dynamically generate decorator classes that depend on in code defined so-called interceptors. Both these methods however have a lot of downsides and although they can solve part of the problems you have, it's still a matter of treating the symptoms (symptoombestrijding). There will always be a lot of problems that those tools will not be able to solve for you the way that SOLID application design can.

christophedemey commented 7 years ago

Hi dotnetjunkie

Thought i'd give you an update : I have not given up! I implemented an example from your linked article today and tommorow i will try and convert my unitofwork test app with the same style and apply the decorator as shown.

Expect more questions! heheh : -)

christophedemey commented 7 years ago

Hi dotnetjunkie

When following your article am i correct in assuming that.

CustomerController is only used to execute the MoveCustomerCommand? Should it not be called MoveCustomerController since it will only handle the MoveCustomerCommand?

So if i would have a DeleteCustomerCommand i would create a DeleteCustomerController which would get a new instance of ICommandHandler<DeleteCustomerCommand> injected?

Concerning the violation of the RAP, if i dont create interfaces wont my integration/unit testing be more difficult as i cannot mock the objects?

Thanks.

dotnetjunkie commented 7 years ago

Should it not be called MoveCustomerController since it will only handle the MoveCustomerCommand?

That depends. In case you're building a Web API, chances are that having controllers will be completely redundant, because every controller would only dispatch to an underlying handler and each controller duplicates the exact same infrastructure. In that case, you're better off doing something as described in this article and shown in this sample application.

In case you're building MVC applications, controllers typically need a bit more custom code, which prevents them from being reduced to a thin -never changing- strip of infrastructure. In that case you can still have one controller per command if you wish, but you can also group multiple use cases in one single MVC controller.

Trick here however is to keep controllers focused as well. Single Responsibility Principle still applies here. If your controllers contain many dependencies (with a heuristic of more than 5), this typically means that a class is getting too big. This means that you should certainly not place every customer-related use case in the "CustomerController", because you will typically have dozens of use cases (both queries and commands) concerning the customer; your CustomerController would become a big ball of mud.

Concerning the violation of the RAP, if i dont create interfaces wont my integration/unit testing be more difficult as i cannot mock the objects?

The RAP doesn't state that you shouldn't create interfaces, what it states is that interfaces are meaningless if you only ever have one implementation. Fakes and mocks can be considered a second implementation of such abstraction, but more importantly is that "discovery of abstractions implies that abstractions are used more than once". We typically fail to see the similarity between parts in our system. Every system allows its users to 'execute' operations/mutations on the system; why not model them around the same abstraction? Every system allows its users to query for data, without causing side effects; why not model those query operations around the same abstraction? We can repeat this question over and over again for other parts of the system, such as code that validates data, and code that checks for permissions, or filters data based on user permissions.

We'll end up with a few generic abstractions that are implemented by probably 80% to 95% of our application's components: that's the RAP in play.

christophedemey commented 7 years ago

Hi dotnetjunkie

I understand first part, you can indeed decide to put another layer on top or not.

//Additional customer controller layer on top.
var controller = container.GetInstance<CustomerController>();
controller.MoveCustomer(5, "woop");
//No customer layer.
var controller = container.GetInstance<ICommandHandler<MoveCustomerCommandModel>>();
controller.Handle(new MoveCustomerCommandModel() { CustomerId = 5, NewAddress = "woop" });

For the second part of the answer.

I can image a scenario where i need to test some functionality which takes a :

So if i feel the need to create an abstraction to make testing easier it is not a problem because mocking it is a second implementation of this abstraction, hence i am not violation the RAP.

Correct ?

Sorry if this feels repetitive but i really want to be sure i understand correctly.

Thanks.

dotnetjunkie commented 7 years ago

I understand first part, you can indeed decide to put another layer on top or not.

Well, I rather mean the following:

protected override async Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, CancellationToken cancellationToken)
{
    string commandName = request.GetRouteData().Values["command"].ToString();
    string commandData = await request.Content.ReadAsStringAsync();
    Type commandType = this.commandTypes[commandName];
    dynamic command = DeserializeCommand(request, commandData, commandType);

    Type handlerType = typeof(ICommandHandler<>).MakeGenericType(commandType);

    dynamic handler = this.handlerFactory.Invoke(handlerType);

    handler.Handle(command);
}

This is a shortened example of this. It is a generic piece of infrastructure that can be plugged in. This is the only piece you have to write. When new commands are created, you don't have to change anything to this infrastructure. You don't have to call GetInstance<T> for your new command handler anymore.

FilesystemController and PLCController

If FilesystemController and PLCController are root types, you typically don't need an abstraction. Root types are the types that are directly resolved by the container; they are not a dependency on anything else. Web API and MVC Controllers are a good example of this. Note that to be able to test a particular class, that class itself doesn't have to have an abstraction. It only needs an abstraction if you wish to mock, decorate, replace or intercept such type, or to break dependencies between modules and layers.

hence i am not violation the RAP

Correct.

There's discussion whether mock implementations should be counted as implementations that satisfy the RAP. Important to note however is that:

  1. Just as with the SOLID principles, you can't always satisfy every piece of code to the RAP
  2. In general, if everything component has an interface that duplicates its name but prefixed with an "I" (such as ProductService + IProductService), that is a blatant violation of the RAP. Instead find common abstractions such as ICommandHandler<T>, IQueryHandler<T, R>, etc).
christophedemey commented 7 years ago

Hi dotnetjunkie

Then what i described is a blatant violation of the RAP :( But the RAP would then instruct me to write tightly coupled code ?

I am trying to solve the scoped puzzle in my unitofwork repository pattern test app but cant seem to find the light. Your explanation makes sense for the command pattern but not for this simple repository and unitof work example or i misimplemented it, i did it following a pluralsight course so tought id be fine :P Can you steer me into the right direction or should i just use the command pattern instead. https://github.com/christophedemey/DI-SimpleInjector-UnitOfWork-Repository-Pattern

dotnetjunkie commented 7 years ago

When it comes to using scoped registrations (i.e. instances that are reused throughout a logical scope), you need a boundary on which you can supply a scope. When using a web application that boundary typically is a web request, and when you use Simple Injector, its integration packages allow scopes to be applied transparently for you. If you wish to apply a scope in a different type of application, or on a different level, you typically need to supply the scope yourself, but to be able to do this, you need to have the correct 'interception point'. A ICommandHandler<T> is an ideal interception point, because it describes an atomic business operation; that's typically where you wish to apply your scoping. Within such operation, you wish to reuse the DbContext.

In other words:

In your example application, you are running a console application. Typically, a console application is something that runs for a short period of time, and the complete execution could be considered to be a single 'request'. In case you want to do it 'more often', this typically means you start the console application again.

In such case it typically means that in the Main method you create a scope (for instance ThreadScopedLifestyle) for the duration of the application; the application's duration is a single request. Another option is to register 'scoped' instances as Singleton, since scoped effectively means singleton when an application only handles one single request and dies.

Other console applications are long running processes. This can happen when they listen to some queue and process items from the queue as they come in. (This however is a more likely scenario for a Windows Service). In such case, the handling of a single item from the queue is typically considered to be a single request. Thus, the handling of an item is typically wrapped in a scope. This allows scoped instances to be reused within that single request, while other requests get their own scope + scoped instances.

christophedemey commented 7 years ago

Hi dotnetjunkie

In my job we create mostly windows services and they always are a console application with the topshelf framework on top to make them installable/runnable as a service.

In this demo app i was thinking of a service -> queue based processing system which would instantiate a unit of work which gets its context and then passes it on to it's repository(s).

The command pattern that you described in your article could be easily implemented here and resolve my questions regarding this scoped lifestyle.

Thanks for your time.