When to move a common field into a base class?

Question

I currently have two derived classes, A and B, that both have a field in common and I'm trying to determine if it should go up into the base class.

It is never referenced from the base class, and say if at some point down the road another class is derived, C, that doesn't have a _field1, then wouldn't the principal of "least privileged" (or something) be violated if it was?

public abstract class Base
{
    // Should _field1 be brought up to Base?
    //protected int Field1 { get; set; }
}

public class A : Base
{
    private int _field1;
}

public class B : Base
{
    private int _field1;
}

public class C : Base
{
    // Doesn't have/reference _field1
}

Answer 1

It all depends upon the exact problem you're trying to solve.

Consider a concrete example: your abstract base class is Vehicle and you currently have the concrete implementations Bicycle and Car. You're considering moving numberOfWheels from Bicycle and Car to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat class then it's going to break.

Now, if your abstract base class was WheeledVehicle then it's logical to have the numberOfWheels member variable in there.

You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.

Answer 2

Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two.  @Pete hints at this without fully going there.

Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it.  Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).

This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.

Answer 3

I'm going to play devil's advocate here.

Right now you should do nothing.

Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.

When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.

Answer 4

In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.

I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.

An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.