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.

@Rig you are bordering on blasphemy in these parts with a comment like that.
–
RyathalAug 28 '12 at 15:47

29

I'm ok with that. I've read the book and can have my own opinion on it.
–
RigAug 28 '12 at 15:54

6

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.
–
Simon WhiteheadAug 29 '12 at 2:36

11 Answers
11

They tend to lead to YAGNI issues. Unless you have a descendant class that actually does stuff with the protected member, make it private.

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.

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.

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.

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.
–
MatsemannAug 28 '12 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.
–
Robert HarveyAug 28 '12 at 16:18

4

@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.
–
TelastynAug 28 '12 at 16:32

@Telastyn - not "necessarily". Sometimes, an inheritance hierarchy is defined all in one go and within a single file (where the language doesn't force different classes into different files, at least). This tends to be a design pattern thing rather than an extensibility thing - of course OOP design patterns are intended to be extensible, but they can also be used in cases where extensibility seems unlikely to be an issue, for familiarity or other reasons. Vertical distance is still an issue if the classes are complex, of course - and huge files are a problem too, at least to some people.
–
Steve314Aug 28 '12 at 21:46

2

@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.
–
TelastynAug 28 '12 at 22:08

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.

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:

You created a public property with protected setter. The protected field should be solved with a protected property with private setter.
–
AndyAug 28 '12 at 22:32

1

+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.
–
AndyAug 28 '12 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
–
Martin BeckettAug 30 '12 at 4:42

1

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.
–
supercatOct 8 '12 at 23:33

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.

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".

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?

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 (http://www.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.

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).

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.
–
AndyAug 28 '12 at 22:35

1

@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.
–
m3th0dmanAug 29 '12 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.
–
AndyAug 29 '12 at 11:31

1

@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.
–
m3th0dmanAug 30 '12 at 6:41

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.
–
AndyAug 30 '12 at 14:39

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.

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.

I think the suggestion is to use private instead. I have always wished that Java variables were private by default, but the default is more like "protected" because they are accessible by other classes in the package (but not the sub-class). Usually, this kind of sharing indicates you have not divided functionality appropriately between the classes in that package.

Since Scala came along, I've wished that Java variables and collections were also immutable by default. It's nice to have protected and public variables and mutable collections WHEN YOU NEED THEM, but it's even better if they are private and mutable by default so that you have to think before getting yourself into trouble.