That's not a type, that's a named instance.

I was recently shown some code where the developer had an abstract class where all of the functionality existed in the base class and was controlled strictly by the constructor arguments, something like this


public abstract class BaseWidget:IWidget
{
  protected BaseWidget(string configurationValue1, SomeEnum configurationValue2, int configurationValue3)
  {
    //set private fields to those values
  }
  public void DoSomethingWidgetRelated()
  {
    //this method uses the "configuration" fields to perform some logic
  }
}

Each concrete "implementation" of this class simply passed arguments to the base type via the constructor... and that's all.


public class FooWidget:BaseWidget
{
  public FooWidget():base("fooValue",SomeEnum.Foo,47){}
}

When clicking through the codebase looking for a random implementation, it actually appeared as if there was a startling number of these "empty" types. This is definitely a "code smell".

They have "invented" the named instance. This doesn't need to be a type, this is configuration of a type. The class FooWidget doesn't need to exist in my opinion, it's not doing anything for you other than bloating your namespace and giving you a few more (trivial) lines to maintain. Odds are you didn't even write tests around them (what would you test? that parameters are being passed?) Most DI libraries will happily let you configure all the instances you want with whatever constructor arguments you want, no inheritance or namespace clutter required. (I suspect that this might have also been a case of a "custom" DI container, which might have been part of the problem, I won't go into that)

In this example, first I would convert BaseWidget to just a normal concrete class.


public class Widget:IWidget
{
  public Widget(string configurationValue1, SomeEnum configurationValue2, int configurationValue3)// or maybe internal if letting other types new this up is *really* a problem, but I suspect it's not.
  {
    //set private fields to those values
  }
  public void DoSomethingWidgetRelated()
  {
    //this method uses the "configuration" fields to perform some logic
  }
}

And then just configure your instances in DI with relevant values, in Ninject, for example


public class WidgetModule:NinjectModule
{
  public void Load()
  {
    Kernel.Bind<IWidget>()
      .To<Widget>()
      .InTransientScope()//or whatever scope is relevant
      .WithConstructorArgument("configurationValue1","fooValue")
      .WithConstructorArgument("configurationValue2",SomeEnum.Foo)
      .WithConstructorArgument("configurationValue3",47);

    Kernel.Bind<ISomethingThatDependsOnIWidget>()
      .To<Something>();
  }
}

"But what if I have multiple implementations and we need to specify what goes where?" I hear your screaming.

That's what names are for.


public class WidgetModule:NinjectModule
{
  public void Load()
  {
    Kernel.Bind<IWidget>()
      .To<Widget>()
      .InTransientScope()//or whatever scope is relevant
      .WithConstructorArgument("configurationValue1","fooValue")
      .WithConstructorArgument("configurationValue2",SomeEnum.Foo)
      .WithConstructorArgument("configurationValue3",47)
      .Named("FooWidget");

    Kernel.Bind<ISomethingThatDependsOnIWidget>()
      .To<Something>()
      .WithConstructorArgument("widget",c=>c.Kernel.Get<IWidget>("FooWidget"));

    Kernel.Bind<ISomethingElseThatDependsOnIWidget>()
      .To<SomethingElse>()
      .WithConstructorArgument("widget",c=>c.Kernel.Get<IWidget>("SomeOtherWidget"));
  }
}

So what's the point? The point is to reduce the number of classes in your assembly, and move all your configuration to a central location (your DI configuration). You end up with one or more DI modules/registries etc taking the place of what (in this case) was a bunch of do-nothing classes.

The other point that this drives home is the idea of preferring composition over inheritance. As great as the features of OOP are, working in any non trivial codebase with a lot of deep inheritance will damage even a grizzled veterans sanity. If you're really designing SOLID classes (emphasis on the I and D) my version will make a lot more sense since you are likely passing in interfaces not just the configuration values in my trivial example.

Finally, if you have an implementation like this I think there's a strong possibility that BaseWidget is a bit of a god class. Possibly it's something that needs to be broken up into some differing strategies and have those injected as well.

This is all just my opinion, of course, but I have noticed a trend lately where lots of teams are "using DI" in C# for some things, but still not really getting the mileage out of it that they should be.

Menu