"Base class params are not always used" code smell

Suppose you had such code:

public Base
{
   abstract void Register();
}

public Registrator1: Base
{
   override void Register()
   {
      //uses the current state of the object to populate the UI captions
   }
}

public Registrator2: Base
{
   override void Register()
   {
      //uses the current state of the object to populate the UI captions
   }
}

But When you receive a new business rule asking you to write Registrator3 which actually registers based on some parameter and you change your code base to the next:

public Base
{
   abstract void Register(externalParam);
}

public Registrator1: Base
{
   override void Register(externalParam)
   {
      //uses the current state of the object to populate theUI
   }
}

public Registrator开发者_运维技巧2: Base
{
   override void Register(externalParam)
   {
      //uses the current state of the object to populate the UI
   }
}

public Registrator3: Base
{
   override void Register(externalParam)
   {
     //uses a DDD - service passed in the params to populate the UI
   }
}

But Registrator1 and Registrator2 do not need that param and the code becomes smelly. What are the ways to re-write this code?

---

You could use an object as a parameter here; which is commonly used in scenarios where the number of parameters can vary depending on the call being used.

struct RegistrationInfo
{
    public static readonly RegistrationInfo Empty = new RegistrationInfo();
    public string Username;
    public string CustomerName;
    public string Validity; 
}

abstract class Base
{
    public abstract void Register(RegistrationInfo info);
    // If you want to retain the paramaterless call:
    public void Register()
    {
         Register(RegistrationInfo.Empty);
    }
}

class Registrar1 : Base
{
    public override void Register(RegistrationInfo info)
    {
        if (info.Username == null) throw new ArgumentNullException("info.Username");
    }
}

class Registrar2 : Base
{
    public override void Register(RegistrationInfo info)
    {
        if (info.CustomerName == null) throw new ArgumentNullException("info.CustomerName");
    }
}

This has the advantage that you don't need to change method parameters (which is breaking interface) each time a parameter is added. The usage also becomes somewhat self-documenting:

var r = new Registrar1();
r.Register(new RegistrationInfo(){ Username = "JimJoe" });
r.Register(RegistrationInfo.Empty);

It's like air freshener for this type of code smell, while it's still smelly; you can make it smell nicer.

Finally you can make the call-site cleaner by making it a params argument (this has a small amount of overhead); in all honesty though it is more smelly because it's a language hack. Finally you could improve it with generics:

class RegistrationInfo
{

}

class RegistrationInfo1 : RegistrationInfo
{
    public string Arg;
}

class RegistrationInfo2 : RegistrationInfo
{
    public int Arg;
}

interface IBase<in TRegistration>
    where TRegistration : RegistrationInfo
{
    void Register(TRegistration registration);
}

class Base : IBase<RegistrationInfo>
{
    public void Register(RegistrationInfo registration)
    {

    }
}

class Registrar1 : IBase<RegistrationInfo1>
{
    public void Register(RegistrationInfo1 arg)
    {
    }
}

class Registrar2 : IBase<RegistrationInfo2>
{
    public void Register(RegistrationInfo2 arg)
    {
    }
}

---

Is it not possible to contain the logic for externalParam in Registrator3? In other words, Registrator3 uses the param, then calls the unmodified parameterless base?

A lot really depends on where the logic belongs. If it is something intrinsic to the base, then put it in the base, and either overload the Register() function or supply a default value for the param so that sub classes don't need to provide it.

---

Assuming you want to reuse the registration logic from the base class, you could update the code as follows:

public class Base
{
   public virtual void Register(object externalParam)
   {
       // base registration logic goes here
   }
}

public class Registrator1: Base
{
   public override void Register(object externalParam)
   {
       base.Register(null);
       // custom registration logic goes here
   }
}

public class Registrator2: Base
{
   public override void Register(object externalParam)
   {
       base.Register(null);
       // custom registration logic goes here
   }
}

public class Registrator3: Base
{
   public override void Register(object externalParam)
   {
       base.Register(externalParam);
       // custom registration logic goes here
   }
}

HTH,

Cosmin

EDIT: Updated code to compile.