0

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?

5
  • "Clean" coding is really a phrase for a very loose collection of guidelines and advice that is generally considered to be "a good idea". Clicking through your link, it seems that you're asking a question based on a very specific book called "Clean Code: A Handbook of Agile Software Craftsmanship". That's an important distinction to make here as one book does not speak for the entire realm of "clean" coding guidelines.
    – Flater
    Nov 1, 2023 at 5:18
  • 3
    The example used here is so vague that it can't be concretely discussed. Without examining how this field is being used (and where), it's impossible to actually try and apply (or counter) your interpretation of this advice.
    – Flater
    Nov 1, 2023 at 5:18
  • yeah, why do you need to access the id?
    – Ewan
    Nov 1, 2023 at 17:28
  • Perhaps the id is not the best example. Flag instance variables are often better candidates to be protected within a hierarchy like this. Sure, we could implement one or two methods to toggle the flag but, Is it worth it? And, before any of you rush to say "of course because encapsulation bla bla bla" think first if you are one of those lombok lovers who hate writing setter and getters-
    – Laiv
    Nov 2, 2023 at 8:52
  • "Does Clean Code suggest avoiding all base class instance variables" - No. Think of what's said in the original text as of a preference rather than a hard rule. That is, as you're decoupling your system into software elements (functions, classes, bundles of classes, components) with a narrow, well-defined job (responsibility), maintain a preference for readability and cohesion - as in, keep related things close (specifically, things related to that responsibility) so that you can figure out what's going on within that one context when reading, and keep things together when they change together Nov 2, 2023 at 14:38

2 Answers 2

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.

2
  • "in historical versions of Java it wasn't possible to implement interfaces using properties" - I'm not aware of any version of Java having a concept of "property". Could you elaborate what you mean? Nov 2, 2023 at 8:48
  • A instance level variable marked as public.
    – DavidT
    Nov 2, 2023 at 10:43
1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.