2

I know there is all this talk about having only one composition root that set all your dependencies and that you should always prefer constructor injection to other types, but sometimes it just doesn't make any sense for me. Here is my pseudo code :

    public class FakeModel
    {
        private FakeDependency Dependency = ServiceLocator.Get<FakeDependency>();

        public string Name { get; private set; }

        public FakeModel(string name)
        {
            Name = name;
        }

        public void UpdateName(string name)
        {
            Dependency.RandomAction();
            Name = name;
        }
    }

Special considerations :

  • FakeModel and FakeDependency sit in a class library project that is reused my many other projects.
  • All the other projects using this class library doesn't know, and shouldn't even know, that FakeDependency exists.
  • There will always be only one (+test) implementation of FakeDependency.
  • The only reason to replace the FakeDependcy is to run tests that only test the class library.

Would you rewrite this code differently, or it makes sense considering the requirements?

EDIT

There seems to be some confusion about the dependency, the service locator and how to use the class library. Think about the model as the public interface of this class library. Everything else is hidden from other projects, is undocumented and subject to changes anytime. You should not ever used any other methods than those define by the model.

About the service locator; it lives inside the class library project. It's only purpose is to make it possible to test the class library and absolutely not to inject new dependencies into this library. If I wanted to make it possible to replace the dependency I would of course have used constructor injection like that public FakeModel(string name, FakeDependency dependency) but that is not the case.

Then how do you use this class library currently? Just like that :

public void Test()
{
    var model = new FakeModel("MyName");
    model.UpdateName("NewName");
}

The class library should be self contained. Maybe using the word "dependency" is confusing as it's not something that you need to provide to the class library.

  • 1
    The problem with service locators remains unchanged. If the dependency is not found, your application will crash at runtime, and you won't have any idea why. The point of a composition root is that that is the place where things can go wrong. – Magus Jun 4 '15 at 15:33
  • 1
    Why are you using a service locator? It sounds like what you're doing currently is setting up the dependency binding in the same project as FakeDependency lives. That would work just as well with constructor injection. – Ben Aaronson Jun 4 '15 at 15:47
  • I would still prefer to have written this class so that the dependency is injected through the constructor. Also, you say "there will always be only one implementation" and then your very next line illustrates a reason for a second (test) implementation. – Eric King Jun 4 '15 at 15:48
  • @BenAaronson How would you set it through constructor injection without exposing it to the other projects? You make the constructor internal then you could create it via a factory, but then you would just have moved the ServiceLocator into that factory. Does it add any value? – Gudradain Jun 4 '15 at 17:45
  • @Gudradain The project with FakeDependency can expose a module class with a method like ConfigureBindings(IKernel kernel) (or whatever type your IOC container uses). Then in the folder where you do your composition, in your application entry project, you can call that method – Ben Aaronson Jun 4 '15 at 18:17
2

Yes I would rewrite it.

Edit: new code based on your expanded comments and question:

[assembly: InternalsVisibleTo("TestAssembly")]
    public class FakeModel
    {
        private IFakeDependency dependency;
        public string Name { get; private set; }

        public FakeModel(string name)
        {
            Name = name;
            this.dependency = new FakeDependency();
        }
        //use this constructor in tests
        internal FakeModel(string name, IFakeDependency dependency)
        {
            Name = name;
            this.dependency = dependency;
        }
        public void UpdateName(string name)
        {
            dependency.RandomAction();
            Name = name;
        }
    }
  • I think the question is then should FakeDependency be bound in the application composition point in the project referencing this one, or in the project which it lives? From my understanding, OP is saying that the former is the standard way of doing this, but has the issues that it means that now FakeDependency has to be public and known about by other projects. And also that this same binding has to be repeated in lots of projects. – Ben Aaronson Jun 4 '15 at 16:57
  • hmm I shall add more code.. – Ewan Jun 4 '15 at 17:01
  • @BenAaronson Yup you got it. In fact, FakeDependency should never be used outside the class library project. The Model provides the public interface of the library project and all the other stuffs should stay hidden. – Gudradain Jun 4 '15 at 17:11
  • Ewan, the service locator in the example is a part of the class library project. And no, I don't want to make it easy to change in the sense that you can provide another implementation if you use that class library. If you exclude the tests, there is and should only be one implementation. And all other project using this library shouldn't even bother knowing it. – Gudradain Jun 4 '15 at 17:13
  • I think you will end up exposing it either way. maybe you could add your ServiceLocator code? – Ewan Jun 4 '15 at 17:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.