Regarding to Type in C#

Question

I tried to use C# DI method to implement something. following is my code snippet.

public interface IMessageService
{
    void Send(string uid, string password);
}

public class MessageService : IMessageService
{
    public void Send(string uid, string password)
    {
    }
}

public class EmailService : IMessageService
{
    public void Send(string uid, string password)
    { 
    }
}

and code that creates a ServiceLocator:

public static class ServiceLocator
{

    public static object GetService(Type requestedType)
    {
        if (requestedType is IMessageService)
        {
            return new EmailService();
        }
        else
        {
            return null;
        }
    }
}

now, I create a test code with

public class AuthenticationService
{
    private IMessageService msgService;
    public AuthenticationService()
    {
        this.msgService = ServiceLocator
          .GetService(typeof(IMessageService)) as IMessageService;
    }
}

but, it looks like, I always get null returned by GetService() function. Instead I expect to get EmailService object via GetService() function, so how to do it correctly?

For those who will attempt to use such pattern for any reason even for **complex system**, know that Service Locator is an [anti-pattern](http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/) and if you think you need it, rethink your design to get rid of it. — CodeNotFound, Jun 10 '18 at 07:39
@CodeNotFound - Are you saying it is an anti-pattern when it is returning a hard-coded class like this code does? Or are you saying it more generally? If so, why? — Enigmativity, Jun 10 '18 at 07:53
@Enigmativity I'm saying it generally :) Why? for all reasons explained in the blog article in the URL I added to my first comment. Also hard-coded resolution can be mark as a bad practice. — CodeNotFound, Jun 10 '18 at 07:56
@CodeNotFound - I think that's a little too simplistic. A implementation needs only the contract (interface) and the behaviour (unit tests) - given those two things this pattern works well. Add in decorators and dynamic loading and you can make a well tested, flexible development environment. I think it's an anti-pattern if you only go half-way. — Enigmativity, Jun 10 '18 at 08:02
@Enigmativity _and the behaviour (unit tests)_ is lost with `return new EmailService();`. How do you mock that when unit testing the class that use `IMessageService`. Maybe I'm missing something — CodeNotFound, Jun 10 '18 at 08:04
@CodeNotFound - I certainly wouldn't hard-code the creation of types. I'd be more included to register an assembly and use attributes to flag which types can be created. Then I'd use that to instantiate the instances needed. — Enigmativity, Jun 10 '18 at 10:47

zaitsman · Answer 1 · 2018-06-10T07:33:56.927

12

What you are passing in is an instance of Type.

So this condition requestedType is IMessageService is never true.

What you need to do is

public static object GetService(Type requestedType)
{
    if (requestedType == typeof(IMessageService))
    {
        return new EmailService();
    }
    else
    {
        return null;
    }
}

As a side note, this is quite a bad pattern - your so-called service locator has concrete knowledge of concrete types. You're better off using reflection or some traditional registration pattern for IoC to make this generic.

edited Jun 10 '18 at 07:33

answered Jun 10 '18 at 06:54

zaitsman

8,984
6
47
79

2

@CodeNotFound please do not put words in my mouth by editing my answer. The specific implementation of service locator from OP is bad. I don't believe that we should NEVER use it or that it has no place in a complex system. – zaitsman Jun 10 '18 at 07:34
1

You know, this got me thinking... is this _really_ bad? If you're rolling out your own DI/ServiceLocator anyway, and it's not meant to be reusable between projects, what do you gain by splitting it up in two pieces? A generic locator/instantiator and a registry. It's more code, more complex, more spread out and more indirect. Go-to-definition is not enough anymore to find where/how some object is constructed. Possibly even a bit slower if reflection is involved. And you've gained... well, nothing that I can see. It's less legible and has more opportunities for bugs. – Vilx- Jun 10 '18 at 11:21
@Vilx- if you are measuring code coverage, and write the code this way, you will have to write a test for each class that you register. If you are split the logic to a generic and registration, you can then test the resolution once and write another test that ensures all necessary classes are registered. Or, you can even write a reflection based registration process that adds all types based on their interfaces. – zaitsman Jun 10 '18 at 14:37
1

About testing - `write a test for each class that you register` and `[a] test that ensures all necessary classes are registered` - what exactly is the difference here? :) The reflection process I especially dislike because it's super-magic. Unless you're the person who wrote this, everyone else will be left scratching their heads about how exactly is the magic working. That's the thing - I really dislike _magic_ code - something that happens in a mysterious way with no visible lines of code. "It just works" is a nice principle for UX design, but an awful one for code design. – Vilx- Jun 10 '18 at 18:21

Fabjan · Accepted Answer · 2018-06-12T07:34:10.513

I tried to use C# DI method to implement something. following is my code snippet

There is no such pattern called "C# DI method". I presume that our task here is to use a ServiceLocator pattern for DI. Don't do that!

The ServiceLocator is arguably an anti-pattern and leads to maintenance nightmare because class dependencies are hidden. In most real-world scenarios we should avoid using it.

With help of some DI framework such as SimpleInjector (it could be any other well-known DI framework though) you could achieve the same result. However this time the code will be more maintainable and a lot easier to test. For that we could create a Mock<IMessageService> and pass its object to a constructor of EmailService.

But let's get back to the subject and have a look into how we could use Simpleinjector here:

public class AuthenticationService
{
    private readonly IMessageService _msgService;

    public AuthenticationService(IMessageService msgService)
    {
        this._msgService = msgService;
    }
}

In order to use that somewhere in code we need to register this dependency. A minimal code example would be:

var container = new SimpleInjector.Container();
container.Register<IMessageService, EmailService>();
container.Verify();

And that's all it requires!

P.S. This is not an ad of this particular DI framework. Feel free to use any other framework, I've used it in this example because I'm more familiar with it

That is not an answer to the issue of the OP. I upvoted because it explains how to move to a good practice :-) — CodeNotFound, Jun 10 '18 at 08:02
@CodeNotFound Yeah, I just wanted to point to the fact that service locator pattern is a shoddy solution here. Sometimes a good answers are indirect ones... — Fabjan, Jun 10 '18 at 08:04
English seems to be a second language for OP. Maybe edit the question if you want to help. — s3raph86, Jun 10 '18 at 08:44

Regarding to Type in C#

2 Answers2