187

Clean Code suggests avoiding protected variables in the "Vertical Distance" section of the "Formatting" chapter:

Concepts that are closely related should be kept vertically close to each other. Clearly this rule doesn't work for concepts that belong in separate files. But then closely related concepts should not be separated into different files unless you have a very good reason. Indeed, this is one of the reasons that protected variables should be avoided.

What is the reasoning?

15
  • 8
    show an example where you think you need it
    – gnat
    Commented Aug 28, 2012 at 15:11
  • 70
    I wouldn't take much in that book as gospel.
    – rlperez
    Commented Aug 28, 2012 at 15:30
  • 40
    @Rig you are bordering on blasphemy in these parts with a comment like that.
    – Ryathal
    Commented Aug 28, 2012 at 15:47
  • 42
    I'm ok with that. I've read the book and can have my own opinion on it.
    – rlperez
    Commented Aug 28, 2012 at 15:54
  • 12
    I've read the book and although I do agree with most of what's in there, I wouldn't say I consider it gospel either. I think that every situation is different.. and being able to stick to the exact guidelines represented in the book are quite difficult for lots of projects. Commented Aug 29, 2012 at 2:36

10 Answers 10

298

Protected variables should be avoided because:

  1. They tend to lead to YAGNI issues. Unless you have a descendant class that actually does stuff with the protected member, make it private.
  2. They tend to lead to LSP issues. Protected variables generally have some intrinsic invariance associated with them (or else they'd be public). Inheritors then need to maintain those properties, which people can screw up or willfully violate.
  3. They tend to violate OCP. If the base class makes too many assumptions about the protected member, or the inheritor is too flexible with the behavior of the class, it can lead to the base class' behavior being modified by that extension.
  4. They tend to lead to inheritance for extension rather than composition. This tends to lead to tighter coupling, more violations of SRP, more difficult testing, and a slew of other things that fall within the 'favor composition over inheritance' discussion.

But as you see, all of these are 'tend to'. Sometimes a protected member is the most elegant solution. And protected functions tend to have fewer of these issues. But there are a number of things that cause them to be treated with care. With anything that requires that sort of care, people will make mistakes and in the programming world that means bugs and design problems.

13
  • Thanks for many good reasons. However, the book mentions it specifically in a context of formatting and vertical distance, it's that part I'm wondering about.
    – Matsemann
    Commented Aug 28, 2012 at 16:12
  • 2
    It seems that Uncle Bob is referring to vertical distance when someone is referring to a protected member between classes, and not in the same class. Commented Aug 28, 2012 at 16:18
  • 5
    @Matsemann - sure. If I remember the book accurately, that section focuses on the readability and discover-ability of code. If variables and functions work on a concept they should be close in code. Protected members will be used by derived types, which necessarily cannot be close to the protected member since they're in a completely different file.
    – Telastyn
    Commented Aug 28, 2012 at 16:32
  • 3
    @steve314 I can't imagine a scenario where the base class and all of its inheritors will only ever live in one file. Without an example I am inclined to think it an abuse of inheritance or poor abstraction.
    – Telastyn
    Commented Aug 28, 2012 at 22:08
  • 4
    YAGNI=You Aint Gonna Need it LSP=Liskov Substitution Principle OCP=Open/Close Principle SRP=Single Responsibility Principle Commented Dec 11, 2013 at 1:48
61

It's really the same reason you avoid globals, just on a smaller scale. It's because it's hard to find everywhere a variable is being read, or worse, written to, and hard to make the usage consistent. If the usage is limited, like being written in one obvious place in the derived class and read in one obvious place in the base class, then protected variables can make the code more clear and concise. If you're tempted to read and write a variable willy-nilly throughout multiple files, it's better to encapsulate it.

28

I haven't read the book, but I can take a stab at what Uncle Bob meant.

If you put protected on something, that means that a class can inherit it. But member variables are supposed to belong to the class in which they are contained; this is part of basic encapsulation. Putting protected on a member variable breaks encapsulation because now a derived class has access to the implementation details of the base class. It's the same problem that occurs when you make a variable public on an ordinary class.

To correct the problem, you can encapsulate the variable in a protected property, like so:

protected string Name
{
    get { return name; }
    private set { name = value; }
}

This allows name to be safely set from a derived class using a constructor argument, without exposing implementation details of the base class.

4
  • You created a public property with protected setter. The protected field should be solved with a protected property with private setter.
    – Andy
    Commented Aug 28, 2012 at 22:32
  • 2
    +1 now. Setter could be protected too I suppose, but the point ids the base can enforce if the new value is valid or not.
    – Andy
    Commented Aug 28, 2012 at 22:39
  • Just to offer an opposing point of view - I make all my variables protected in a base class. If they are only to be accessed through a public interface then it shouldn't be a derived class, it should just contain a baseclass object. But I also avoid hierarchies more than 2 classes deep Commented Aug 30, 2012 at 4:42
  • 2
    There is a huge difference between protected and public members. If BaseType exposes a public member, that implies that all derived types must have that same member work the same way, since a DerivedType instance may be passed to code which receives a BaseType reference and expects to use that member. By contrast, if BaseType exposes a protected member, then DerivedType may expect to access that member in base, but base can't be anything other than a BaseType.
    – supercat
    Commented Oct 8, 2012 at 23:33
20

Uncle Bob's argument is primarily one of distance: if you have a concept that is important to a class, bundle the concept together with the class in that file. Not separated across two files on the disk.

Protected member variables are scattered in two places, and, kinda looks like magic. You reference this variable, yet it isn't defined here... where is it defined? And thus the hunt begins. Better to avoid protected altogether, his argument.

Now I don't believe that rule needs to be obeyed to the letter all the time (like: thou shalt not use protected). Look at the spirit of what he is getting at... bundle related things together into one file - use programming techniques and features to do that. I would recommend that you don't over-analyze and get caught up in the details on this.

9

Some very good answers already here. But one thing to add:

In most modern OO languages it is a good habit (in Java AFAIK its necessary) to put each class into its own file (please don't get nitty about C++ where you often have two files).

So it is virtually impossible to keep the functions in a derived class acessing a protected variable of a base class "vertically close" in source code to the variable definition. But ideally they should be kept there, since protected variables are intended for internal use, which makes functions acessing them often "a closely related concept".

3
  • 2
    Not in Swift. Interestingly, Swift doesn’t have “protected” but it has “fileprivate” which replaces both “protected” and C++ “friend”.
    – gnasher729
    Commented Feb 27, 2018 at 15:25
  • @gnasher729 - of course, Swift has a lot of Smalltalk in its heritage. Smalltalk doesn't have anything other than private variables and public methods. And private in Smalltalk means private: two objects even of the same class can't access each other's variables.
    – Jules
    Commented Feb 27, 2018 at 15:46
  • In Flutter (in Dart) (which postdates the answer by some time) "class MyHomePage extends StatefulWidget" and "class _MyHomePageState extends State<MyHomePage>" would always go in the same file, to give another example of a language/system that puts multiple classes in one file. Modern Ada is a fully OO language, and still thinks in terms of packages; a package has to go in its own file, and doesn't necessarily contain only one class. (Actually interface and implementation files, like C++, but as you said, that's beside the point.)
    – prosfilaes
    Commented Feb 23, 2022 at 16:00
6

It's an interesting discussion.

To be frank, good tools can mitigate many of these "vertical" issues. In fact, in my opinion the notion of the "file" is actually something that hinders software development - something the Light Table project is working on (https://chris-granger.com/2012/04/12/light-table-a-new-ide-concept/).

Take files out of the picture, and the protected scope becomes a lot more appealing. The protected concept becomes most clear in langauges like SmallTalk - where any inheritance simply extends the original concept of a class. It should do everything, and have everything, that the parent class did.

In my opinion, class hierarchies should be as shallow as possible, with most extensions coming from composition. In such models, I don't see any reasons for private variables to not be made protected instead. If the extended class should represent an extension including all behaviour of the parent class (which I believe it should, and therefore believe private methods are inherently bad), why should the extended class not represent the storage of such data as well?

To put this another way - in any instance where I feel a private variable would suit better than a protected one, inheritence is not the solution to the problem in the first place.

5

The basic idea is that a "field" (instance-level variable) that is declared as protected is probably more visible than it has to be, and less "protected" than you might like. There is no access modifier in C/C++/Java/C# that is the equivalent of "accessible only by child classes within the same assembly", thus granting you the ability to define your own children that can access the field in your assembly, but not allowing children created in other assemblies the same access; C# has internal and protected modifiers, but combining them makes the access "internal or protected", not "internal and protected". So, a field that is protected is accessible by any child, whether you wrote that child or someone else did. Protected is thus an open door to a hacker.

Also, fields by their definition have pretty much no validation inherent in changing them. in C# you can make one readonly, which makes value types effectively constant and reference types unable to be reinitialized (but still very mutable), but that's about it. As such, even protected, your children (which you cannot trust) have access to this field and can set it to something invalid, making the object's state inconsistent (something to be avoided).

The accepted way to work with fields is to make them private and access them with a property, and/or a getter and setter method. If all consumers of the class need the value, make the getter (at least) public. If only children need it, make the getter protected.

Another approach that answers the question is to ask yourself; why does code in a child method need the ability to modify my state data directly? What does that say about that code? That's the "vertical distance" argument on its face. If there is code in a child that must directly alter parent state, then maybe that code should belong to the parent in the first place?

1
1

I find using protected variables for JUnit tests can be useful. If you make them private then you cannot access them during testing without reflection. This can be useful if you are testing a complex process through many state changes.

You could make them private, with protected getter methods, but if the variables are only going to be used externally during a JUnit test, I'm not sure that's a better solution.

0

As it says there, this is only one of the reasons, that is, logically linked code should be placed inside physical linked entities (files, packages and others). On a smaller scale this is the same as the reason you do not put a class which makes DB queries inside the package with classes that displays the UI.

The main reason however that protected variables are not recommended is because they tend to break encapsulation. Variables and methods should have as limited visibility as possible; for further reference see Joshua Bloch's Effective Java, Item 13 - "Minimize the accessibility of classes and members".

However, you should not take all of this ad litteram, only as a guildline; if protected variables are very bad they would not have been placed inside the language in the first place. A reasonable place for protected fields imho is inside the base class from a testing framework (integration test classes extend it).

5
  • 1
    Don't assume that because a language allows it, it's ok to do. I'm not sure there really is a god reason to allow protected fields; we actually disallow them at my employer.
    – Andy
    Commented Aug 28, 2012 at 22:35
  • 2
    @Andy You haven't read what I said; I said that protected fields are not recommended and reasons for this. There clearly are scenarios in which protected variables are needed; you may want a constant final value only in the subclasses.
    – Random42
    Commented Aug 29, 2012 at 9:33
  • You may want to reword some of your post then, as you seem to use variable and field interchangably and make explicit that you'd consider things like protected consts an exception.
    – Andy
    Commented Aug 29, 2012 at 11:31
  • 3
    @Andy It's pretty obvious that since I was referring to protected variables I was not talking about local or parameter variables but to fields. I also mentioned a scenario where non-constant protected variables are useful; imho it's wrong for a company to enforce the way the programmers write code.
    – Random42
    Commented Aug 30, 2012 at 6:41
  • 1
    If it was obvious I wouldn't have been confused and downvoted you. The rule was made by our senior developers, as was our decision to run StyleCop, FxCop and require unit tests and code reviews.
    – Andy
    Commented Aug 30, 2012 at 14:39
-1

Yes, I agree that is somewhat strange given that (as I recall) the book also discusses all non-private variables as something to be avoided.

I think the issue with the protected modifier is that not only is the member exposed to subclasses it's also made visible to the entire package. I think it is this dual aspect that Uncle Bob is taking exception to here. Of course, one can't always get around having protected methods, and thus the protected variable qualification.

I think a more interesting approach is to make everything public, that'll force you to have small classes, which in turn renders the whole vertical distance metric somewhat moot.

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