CODEWITHSUNDEEP
c#dependency-injection
Kai
Kai

Reputation: 2013

Dependency Injection and other constructor parameters - bad practice?

At the moment I am experimenting a little bit with dependency injection containers, this time with Unity.

Given the following interface:

public interface IPodcastCommService
{
    void Download();

    void Upload();
}

and the following implementation: 

public class PodcastService
{
     private IPodcastCommService commservice;
     private String url;

     public PodcastService(String url, IPodcastCommService commservice)
     {
         this.commservice = commservice;
         this.url = url;
     }
}

Because of the constructor, I was looking for a solution to pass the parameter to it and found it: 

var p = container.Resolve<IPodcastCommService>(new ParameterOverride("url", myUrl));

So far so good, but at the same time I read about how bad this is and how bad the design of the class is and yes, it looks a little bit ugly. But how can I pass a parameter to class in an elegant way?

My first thought was to do it as a property, but then I have to check every time I need the Url that it is already given.

Update: One example, where I read that this is bad design, is this:

But there may be cases where you have pass in custom constructor parameters for the resolve operation. Some may argue that this screams of bad architecture but there’s situations like bringing a DI-container to a legacy system which may require these kind of actions.

Source: http://mikaelkoskinen.net/unity-passing-constructor-parameters-to-resolve/

Upvotes: 15

Views: 12678

Answers (4)

Fendy
Fendy

Reputation: 4643

I don't get it why you need PodcastService with composition of IPodcastCommService, instead of implemented IPodcastCommService and has the url injected by string. I don't understand why your design is bad. Injecting url is good IMHO.

If you think of a better way, I think it can be replaced by injecting a context / configuration instead of a native data type.

public class PodcastService
{
     private IPodcastCommService commservice;
     private IConnectionContext connection;

     public PodcastService(IConnectionContext connection, IPodcastCommService commservice)
     {
         this.commservice = commservice;
         this.connection= connection;
     }
}

public interface IConnectionContext{
    string PodcastServiceUrl{get;}
}

But again, I don't find any benefit from it (except you can handle session / constants / static fields) from the regular approach.

UPDATE:

I have found similiar question about the bad design here. In summary, it is not that native type parameter (string, etc) or custom constructor parameter is bad. It is just that you need to put the parameter to a class that really responsible for the parameter. And custom constructor parameter will be needed if you handle an if-else condition inside an abstract factory pattern.

Upvotes: 3

Egi
Egi

Reputation: 1256

Maybe a

container.RegisterType<PodcastService>(new InjectionConstructor("myUrlParameter"));

would do better, wouldn't it?

but if you need more then one podcastservice and they need another url, parameteroverride is ok I guess.

Upvotes: 1

James
James

Reputation: 82096

In your scenario I think DI via the constructor is perfectly fine. The reason why it's considered a better approach to go via the properties is because it's better for readability i.e. imagine what your constructor would look like if you had to inject 20 properties.

If you only intend on injecting a couple of properties then there is absolutely no harm is doing what your doing. I would considering moving to a property-type approach if you find your dependencies starting to creep up.

Upvotes: 1

user1614543
user1614543

Reputation: 111

It depends on the framework yu are using. For example asp.net mvc providers integration point for IoC containers like DependecyResolver. You should place all your logic for constructing objects and injecting dependencies there. If you are using aps.net you may have some kind of base page that of preinit eevnt injects dependencies. You can't use constructor injection with asp.net, only property injection. With winforms you can use some kind of forms factory to build form objects.

Upvotes: -3

Related Questions