Does Clean Code suggest avoiding all base class instance variables (include private instance variables)?

Question

According to Why is Clean Code suggesting avoiding protected variables?, I should avoid protected variables because of the reason : "closely related concepts should not be separated into different files", that means, for example, if all Animal must have an _id field:

public class Animal{
    protected String _id;
    public String getId(){
        return _id;
    }
}

public class Cat extends Animal{
}

public class Dog extends Animal{
}

To avoid protected fields, I should use "private" to limit the access of _id:

public class Animal{
    private String _id;
    public String getId(){
        return _id;
    }
}

public class Cat extends Animal{
    //use this.getId() to access _id
}

public class Dog extends Animal{
    //use this.getId() to access _id
}

However, it seems that it is still not fulfill "closely related concepts should not be separated into different files" because the _id field is a "closely related thing", which is not put in Cat and Dog, so I think it should be:

public interface Animal{
    public String getId();
}

public class Cat implements Animal{
    private String _id;
    public String getId(){
        return _id;
    }
}

public class Dog implements Animal{
    private String _id;
    public String getId(){
        return _id;
    }
}

which delete the instance variables in base class and copy and paste

private String _id;
public String getId(){
    return _id;
}

to all child classes.

I think as all instance variables in base class (include the private one) are "closely related to the child class", which should not separate them from child class, and hence we should avoid all base class instance variables and then copy and paste all base class instance variables to the child class. Is it true? If not, what do I misunderstand about this suggestion?

Answer 1

I agree with Doc Brown's points, however I think the "because it's Java" point deserves a bit more weight.

Specifically in any programming language there are a set of conventions that have some of their roots in the technical limitations of the language - for example (not sure if this has change in current versions of Java, but) in historical versions of Java it wasn't possible to implement interfaces using properties which is one of the reasons for the prevalence of getter and setters.

However these conventions become part of the "principal of least surprise" you are coding in Java so you expect most properties to be private (or maybe protected) and there there will be getters and setters to access them.

In some of the code I have written I have ended up with close to the solution the OP recommended (making Animal an interface). However I named the interface HasId or Identifiable and the interface typically only had the getId method. My reason for doing that is was:

• The usage of Lombok became prevalent - which avoided a lot of boilerplate getters and setters - one could just declare the variables, so it wasn't a major violation of DRY to add id to multiple files.
• Interfaces support multiple inheritance, so I could extend HasId in other more complex interfaces.
• I didn't want to "burn" my single class inheritance just to add id support.

The "closely related code in the same file" concept is top of my list of "concepts I don't really care about" - meaning that I will do it, if no other more important concepts apply. But often there are other concepts which are more important.

TL;DR - I don't believe I have said anything in contradiction to Doc Brown's answer, however because I weighted my factors/principals differently, I ended up with the opposite outcome.

Answer 2

First, let me say you did not cite "closely related concepts should not be separated into different files" correctly, because you left out the second half "unless you have a very good reason".

When you see _id as "closely related" to your classes Cat and Dog (or all other member variables), you see actually Cat and Dog closely related to their base class Animal, hence according to your line or argumentation, they should be in the same file.

The obvious "good reason" here why Cat, Dog and Animal must be in separate files is that you are programming in Java, and not in a language like C#, C++ or Python where you could put a tightly coupled inheritance hierarchy into one file. That does not mean it would be a good idea to do so in those languages, either, but in Java, you don't need any further justification.

This, however, shows only one thing: that the cited guideline (which is not a rule, law, or dogma btw!), will not bring you really further when deciding if a member variable should be part of a base class and if it should be private or protected.

So instead of asking about good reasons for putting things into different files, let us better ask directly about "good reasons" for choosing a place for a member variable and its visibility.

• The good reason for making _id part of the common base class is the DRY principle and simplicity: all the three classes have the common concept of a string ID, with a trivial implementation, hence there is no good reason to duplicate the code into the base class.

  (A reason for not putting _id in the base class would be if one expects different implementation of getId(); in different derived classes).

• A good reason for making "_id" private (instead of protected) is that in the derived classes the value of _id never needs to be changed. Assuming it is initialized once and only once through in the Animal constructor, there is not need to give Cat and Dog write access to the field. That makes it a little bit easier to reason about the code - when you, for example, in a debugging session, come to the point where you need to find out where _id is initialized or changed, there will be only one source code file Animal.java where you have to set your break points and check what happens.

  Of course, when you extend Cat and Dog to the point where the code is most easily and DRY by giving them write access to certain base class members, then that would become a reason to make these members protected.

Note the accepted answer from that other question from 2012 does not even mention the "different files" idea.