hmemcpy / AgentMulder

** THIS PLUGIN IS NO LONGER MAINTAINED. PLEASE FOLLOW ERNICOMMUNITY FOR UPDATES **
https://github.com/ERNICommunity/AgentMulder
MIT License
151 stars 33 forks source link

Raise a warning if a IoC registered class is explicitly instantiated #43

Open arenhag opened 11 years ago

arenhag commented 11 years ago

Based on the thesis that the container should be the preferred method of accessing concrete implementations it would be awesome if Agent Mulder could raise a warning if a class which is found to be registered in the container also is explicitly instantiated somewhere in the code.

Check this blog post for a more thorough explanation: http://blog.diktator.org/index.php/2013/02/06/ioc-containers-vs-resharper-vs-agent-mulder/

hazzik commented 11 years ago

How to deal with unit tests where you instantiate class explicit?

arenhag commented 11 years ago

My thoughts: 1) Perhaps call the registration code of your application in the [TestFixtureSetUp] method and use the container to resolve what ever implementation that you want to test. That way you force yourself to put the registration code under test as well.

2) Maybe provide a way to indicate to Agent Mulder that these explicit instantiations should be disregarded? A .config file shared in the test project perhaps.

In the end I think I'm more partial to solution 1.

hazzik commented 11 years ago

I don't like idea of instantiating objects with IoC in unit tests - it makes unit tests over-complicated. And for some frameworks such as Xunit.net it will slow down the tests (for ex. Xunit.net does not have concept of fixtures and "setup" code executes for each test)

If this feature will be implemented it shall be complete optional.

hmemcpy commented 11 years ago

Thank you very much for the issue, feedback and blog post! Currently, Agent Mulder will always mark the type as being instantiated by a container (if it really does), and will mark it as used.

For this situation, where types are also instantiated manually via a constructor, there are several things I could do, but the best, I think, would be to a) add a hint that this type is also instantiated manually and b) having the gutter icon click suggest to navigate to the instantiation, which can be the container registration.

This would work in the opposite way of what I suggested as a feature to ReSharper - have the ability to navigate to the concrete instantiation(s) from the constructor's parameter. Suppose you get public Foo(IService service). You could invoke a shortcut on the service variable, and navigate to all the places where Foo is being called with a concrete instance, including implicitly, by DI.

In your case it would work the opposite - from a concrete type you could navigate to all places where this type is instantiated, explicit or implicit.

hmemcpy commented 11 years ago

@hazzik I absolutely agree, by the way, regarding IoC usage in tests - I don't use it, and I prefer instantiating my mocks/dependencies manually. If it turns out being a hassle (i.e., I have to instantiate too many types/have a big setup), than this might be a hint that my type is doing too much, and I should consider a refactoring.

arenhag commented 11 years ago

Awesome discussion, I like it! =)

Regarding using the container in tests: I've never used xUnit and wasn't aware that it lacks setup methods, and since Agent Mulder is supposed to be a plugin agnostic to other tooling (apart from R#) I then agree that requiring container usage in the tests might not be the right path. And apart from that - seems to be a matter of taste. ;)

For this situation, where types are also instantiated manually via a constructor, there are several things I could do, > but the best, I think, would be to a) add a hint that this type is also instantiated manually and b) having the gutter > icon click suggest to navigate to the instantiation, which can be the container registration.

Regarding the quote above: sounds reasonable! Is there any possibility to have a configurable option where one can state if one wishes to have it raised as a hint or a warning? Perhaps one could also state which assemblies (test assemblies) that are allowed to perform explicit instantiations?

hmemcpy commented 11 years ago

Sure, a configuration for this sounds reasonable. I'll try to add this as soon as I can...

Thanks again for the suggestion!