What's your take on protected variables?

Protected variables pose some of the same problems public variables do, they expose parts of their class' internal state which can lead to an invalid class state.
If you create a protected variable inside a class each subclass will be able to do anything it wants with that variable, which means the original base class will not be able to check whether this variable remains valid against its class invariant. This of course leads to a class which is much harder to test, and you will probably have a much harder time proving the correctness of that class.

Using an accessor-mutator pair (even if these are protected) inside your base class allows it to do checks on the variable so the class remains valid at all times.

I never use protected in my code. I find that public and private are expressive enough: I either want this to be part of the interface or a detail of implementation, and I don't feel the need to have derived classes have special privileges. On the other hand, I don't use inheritance all that often, so perhaps protected makes more sense in other styles of coding than my own.

It seems silly to make a 'setter' and 'getter' functions for a variable when the derived object is employ in terms of "is-a". Also, making 'setter' functions opens the variable to be manipulated from outside the object! Of course, one could make in the 'setter' protected, but isn't that 'running in a loop'?

1) If you automatically make getters and setters for every non-public variable then you're doing OOP wrong. 2) Protected member functions don't have the same issues that protected member variables do. As long as your protected member functions maintain class invariants, derived classes can't use them to violate class invariants the same way they could with raw access to protected variables.

TODAY... Generally protected variables are a bad idea. Protected functions are great, though, especially protected mutators. They provide a choke point to ensure all usage is correct. There is a tiny performance overhead, on modern CPUs that overhead is measured in the nanoseconds and is usually unimportant.

In 1980's and early 1990's... Protected variables were one of the best costing solutions to a real problem. They could be used with several design patterns (although they weren't named that, yet) to allow base class functionality to be modified by child classes. Since the compiler optimizers were not that great, having a protected variable meant just a single move instruction whereas mutators and accessors would require multiple jumps and generally hurt performance.


I can imagine some creative folks coming up with problems today where protected variables are a good idea, or at least better than the normal alternatives.

Inheritance hierarchies (as opposed to inheritance only of interfaces) is a bit of a blunt instrument to start with. I'm generally pretty distasteful of any design that requires inheritance more than a single level in depth.

But basic inheritance is just hierarchical adaptation of functionality, and that I can, for the most part, live with. Protected instance variables, on the other hand, allow hierarchical mutation of state. And that is just... ugly. Can't think of a better word for it.

The only reasons to put state in a class in the first place, is if you either need to enforce constraints on the value of that state, or take action based on changes to that state. Protected instance variables violate both of those reasons, since the state may be modified without the knowledge of the super-class.

Since the compiler optimizers were not that great, having a protected variable meant just a single move instruction whereas mutators and accessors would require multiple jumps and generally hurt performance.

As always, the world has gone full circle :

http://developer.android.com/guide/practices/performance.html

Google are advocating that programmers should access class field directly, because the Android platform's compiler, at least for now, can't inline acessor/mutator calls.

[source lang="cpp"]
class Shape {
public:
virtual void CalculateArea() = 0;
private: //private!
int x, y;
int width, height;
};

class Circle : public Shape {
public:
void CalculateArea() {
/* Even if my privates were protected, I'd be in trouble. */
}
};
[/source]

Maybe the answer is proper design.

The only time I feel protected members are useful, is with protected functions, as others said (ignoring any performance concerns). Even further, I feel the only functions which should *maybe* be protected, are virtual functions. Any member variable should be private or public. Any non-virtual function should be private or public. Even most virtual functions should be public... but sometimes it's nice to hide them, to not bloat the interface.

A Ctrl+F of the keyword 'protected' in my code base of 209 classes (50k lines of code) finds only two uses of "protected", and both are for a single virtual function in each class (and both are overriding a Qt base-class function. Hmm, odd).

[size=2](It also found a single use of 'goto' =P)

[size=2](It also found a single use of 'goto' =P)

Was it "//goto store - buy burritos"?

I don't really understand the downside of protected variables. Yeah, you can create a subclass and then access the protected members, but that's only for instances of your derived class.

I mean it just sort of strikes me as the same as the "Do not attempt to stop chainsaw with hands or genitals." sticker.

You can only help people so much. After that they're kind of on their own. If someone wants to use inheritance to abuse a class then they're only hurting themselves, aren't they?

That being said, I usually only use inheritance for polymorphism and use virtual base classes, so maybe I don't know what I'm talking about.

I mean it just sort of strikes me as the same as the "Do not attempt to stop chainsaw with hands or genitals." sticker.

I'm afraid that when it comes to software design a lot of people do still tend to do the equivalent of stopping a chainsaw with their private parts.
Better to educate them about it rather than letting them continue, or let them spread the word to chainsaw-newbies that dismemberment is the right way to go, right?

The example code is a straw-man argument. It should look like:
[source lang="cpp"]class Shape{
public:
virtual void CalculateArea();
};
class Rectangle : public Shape{
public:
void CalculateArea(){
return width * height;
}
Rectangle( int w, int h ) : width(w), height(h) {}
private:
int width, height;
};[/source]

In the context of c++, it seems most experts (Scott Meyes, Herb Sutter) think is not a good idea.

Have you looked at the reasons why they don't encourage them? You drop their names here, but ignore their reasoning as to code maintenance, correctness, etc...

It seems silly to make a 'setter' and 'getter' functions for a variable when the derived object is employ in terms of "is-a".[/quote]That's missing the point -- if you've made the variable private but then added a protected setter/getter, then you've written a lot of boilerplate code that doesn't actually do anything.
The idea is to design the classes in such a way where setters/getters aren't required. The functions presented by a class should be some useful abstraction of it's members, not just the members themselves. If a class has a lot of setters/getters, then it's usually a sign that the class doesn't have clear responsibility or reason to change, and is bad OOD.

I've answered this question before:

http://programmers.stackexchange.com/questions/162643/why-is-clean-code-suggesting-avoiding-protected-variables/162657#162657

I know I'm against the fence here, but I'm gonna try anyway .

The example code is a straw-man argument. It should look like:
[source lang="cpp"]class Shape{
public:
virtual void CalculateArea();
};
class Rectangle : public Shape{
public:
void CalculateArea(){
return width * height;
}
Rectangle( int w, int h ) : width(w), height(h) {}
private:
int width, height;
};[/source]

The thing about this is that for every 'Shape child' I have to include the width, height, x, y variables separately. Isn't one of the advance of class hierarchy not having to repeat oneself.

By the way, I do AGREE that using setter/getters isn't a good idea.

The thing about this is that for every 'Shape child' I have to include the width, height, x, y variables separately. Isn't one of the advance of class hierarchy not having to repeat oneself.

Regarding the bold bit -- no. You need to investigate the use of composition vs inheritance. Composition is a tool to allow for code re-use (not repeating yourself). Inheritance is a tool to allow for polymorphism. If you're looking for code re-use, you should default to composition, and only use inheritance where necessary.

Can every shape derivative be descrived with those 4 variables? Bregma hinted earlier that this probably isn't true -- e.g. polygons have a collection of vertices and edges, circles have a radius, squares have an edge-length...

Also, inheritance has more meaning in OOD than "the derived class gets all the base members" -- e.g. when you inherit from a base, you're declaring that the LSP is valid for your derived class. This means that Square is a Rectangle, and Rectangle is a Square, are both invalid statements in OOP! In Math, every square is a rectangle, so you might think, "square is a rectangle, but with a different Area function", but that's not true in OOP.
Inheritance says that any algorithm that works on the base type, must also work on any derived type (i.e. the derived types can be substituted into existing algorithms) -- but if we drop a square into the following function, it will fail the assertion, therefore Square is not a Rectangle.
void Test( Rectangle* rect )
{
rect->SetWidth(10);
rect->SetHeight(2);
assert( rect->Area() == 20 );
}

If you're making a polymorphic base class, you shouldn't include any variables that aren't going to be common to all derived classes.
If you do have variables that are common to all derived classes, you probably don't need to be using polymorphism either...

[quote name='Caffeware' timestamp='1352523153' post='4999541']
The thing about this is that for every 'Shape child' I have to include the width, height, x, y variables separately. Isn't one of the advance of class hierarchy not having to repeat oneself.

Regarding the bold bit -- no. You need to investigate the use of composition vs inheritance. Composition is a tool to allow for code re-use (not repeating yourself). Inheritance is a tool to allow for polymorphism. If you're looking for code re-use, you should default to composition, and only use inheritance where necessary.

Can every shape derivative be descrived with those 4 variables? Bregma hinted earlier that this probably isn't true -- e.g. polygons have a collection of vertices and edges, circles have a radius, squares have an edge-length... If you're making a polymorphic base class, you shouldn't include any variables that aren't going to be common to all derived classes.
If you do have variables that are common to all derived classes, you probably don't need to be using polymorphism either...
[/quote]

I would further stress that the "code reuse" advantage of inheritance is a plus, but should rarely be a major deciding factor in whether or not to employ inheritance. Just because two types might share a few member functions or member variables in their specific implementation doesn't mean one should immediately jump to the conclusion that you need a common base class to "save yourself the trouble of typing those members more than once." That's an abuse of the by-product of inheritance (which I have sadly seen in practice).

Can every shape derivative be descrived with those 4 variables? Bregma hinted earlier that this probably isn't true -- e.g. polygons have a collection of vertices and edges, circles have a radius, squares have an edge-length...

If you're making a polymorphic base class, you shouldn't include any variables that aren't going to be common to all derived classes.
If you do have variables that are common to all derived classes, you probably don't need to be using polymorphism either...

First, thanks for the your time and replies; I really appreciated it.

Now, in Bregma's example, I don't see the 'trouble', since the Circle class would include the necessary variables to calculate it's own area. Yet, the Circle class needs the x, y, width and height variables. I believe it's not in-logic to say the a circle has a width, for example.

"In object-oriented programming (OOP), inheritance is a way to reuse code of existing objects..."
http://en.wikipedia.org/wiki/Inheritance_%28object-oriented_programming%29

I know Wiki is not the best source, but...

"In object-oriented programming (OOP), inheritance is a way to reuse code of existing objects..."
http://en.wikipedia....ed_programming)

I know Wiki is not the best source, but...

Yes, it is a way, but one of many ways. This particular type of inheritance is called "Implementation inheritance" -- where the subclass is taking parts of it's implementation from the base. The other type is "Interface inheritance" -- where the base only contains pure virtual methods, so only an interface is inherited. Often, inheriting a base class involves both types of inheritance occurring at the same time -- e.g. this is what your example does. You need to be aware that in your case "public Shape" is actually doing two distinct things, i.e. inheriting the implementation details, and inheriting the interface.
Other languages try to make this more explicit -- e.g. Java uses [font=courier new,courier,monospace]extends[/font] for implementation inheritance, and [font=courier new,courier,monospace]implements[/font] for interface inheritance.

If you keep scrolling down that wiki link, it explains that "Implementation inheritance is the mechanism whereby a subclass re-uses code in a base class. ... In most quarters, class inheritance for the sole purpose of code reuse has fallen out of favor".
If you search for "inheritance vs composition" or "composition vs inheritance", you'll find a lot of material explaining why composition is preferable to implementation inheritance.

Now, in Bregma's example, I don't see the 'trouble', since the Circle class would include the necessary variables to calculate it's own area. Yet, the Circle class needs the x, y, width and height variables. I believe it's not in-logic to say the a circle has a width, for example.[/quote]The circle requires only a 'radius' value, so either width or height is wasted. The fact that you've got a useless variable in your Circle class is a 'code smell' indicating that the foundations of this design are flawed.
Using interface inheritance and composition, you could use:
class Shape { public: virtual int Area() const = 0; }//interface
class Circle : public Shape { int radius; public: int Area() const { return pi*radius*radius; } };//interface inheritance
struct Bounds { int width, height; int Area() const { return width * height; } };
class Rectangle : public Shape { Bounds b; public: int Area() const { return b.Area(); } };//interface inheritance and compositionNow if you want every Shape to have an x/y position, you could change the above to:
struct Pos { int x, y; };
class Shape { public: virtual int Area() const = 0; virtual Pos Position() const = 0; }//interface
class Circle : public Shape { Pos pos; public: int Pos Position() const { return pos; } ... }...but if every derived class is doing the same thing, you can instead use implementation inheritance for those cases, e.g. struct Pos { int x, y; };
class Shape { public: virtual int Area() const = 0; Pos Position() const { return pos; } protected: Pos pos; }//interface + base implementation
So, in those specific cases (where all derived class must do the same thing), this is ok. However, this should be quite rare in good designs -- inheritance isn't used very often (some OOP classes make it seem like you should be using inheritance every day, but it should actually be rare), especially with large inheritance trees. So given that -- if you're not often using inheritance, then you're not often going to run into the situation where lots of derived classes are doing the same thing, so you won't often have to use implementation inheritance.

Back to the circle -- the problem here is that when the base class defined width/height, it places no invariants on those variables. As far as the base is concerned, those variables can have any value. However, the circle does have an invariant -- the width and height must always be equal (otherwise it would be an ellipse, not a circle).
In practice, you can just deal with this™, but in theory this is a clear violation of OO principles. A derived class cannot add new invariants to the base class's functionality, otherwise you're violating substitutability.

This becomes a much bigger issue when the base class does have invariants, but the derived class has different invariants. Private allows a class designer to completely encapsulate the logic behind maintaining a class's invariants within the logic for that class -- as long as everything is private, if you get an assertion failure indicating an invariant has been violated, then the only place you need to look is at the code for that class. If the members were public, you'd instead have to possibly look at the whole code-base. If the members were protected, you'd instead have to look at all of the code for the class, and the code of any class that inherits from it. Where possible, it's best to use private to tightly contain your class-invariant logic in as small a section of the code-base as possible.

We can also consider here that OOP and OO Design are two different things. You can use an ad-hoc design, and implement it using the tools that OOP languages give you. Or, you can create a design using OO theory, and implement it using a non-OOP language.
i.e. just because you're writing code in an OOP language, that doesn't actually mean that you're using OO theory in your decisions -- lots of "bad" OO code is simply code that's written using OOP but ignores the theory behind it.

Also, as always, you can't properly critique a class, such as Shape, without also knowing what exactly is the problem that it's supposed to solve. In a vacuum with no particular problems in mind, then there's infinite different ways to describe a shape, and none are better than any other. It's only once you're given a specific problem that we can really look at specific ways to implement a solution.
e.g. if the purpose of the program was to simply calculate area values, I'd probably go with:
struct Rect { int w,h; };
struct Circle { int r; };
void CalcAreas( std::vector<int>& out, const std::vector<Rect>& in );
void CalcAreas( std::vector<int>& out, const std::vector<Circle>& in );

By the way, I do AGREE that using setter/getters isn't a good idea.

I don't agree that using setters/getters is a bad idea. Leastwise, it's not in itself wrong, just over-using it is.

Using setters/getters for every single variable, 'just because', is bad. But using setters where the class needs to know when the member variable changed is important, and using getters when whatever you are getting needs to be calculated, is required.

The thing about this is that for every 'Shape child' I have to include the width, height, x, y variables separately...[/quote]
Hodgeman rightly mentioned the warning bells that emerge when "Circle" has to ignore either width or height. The warning bells are actually telling you that Circle doesn't have a width or a height, it has a radius or a diameter. You use the diameter to "calculate" the circle's width or height - except that the diameter is exactly equal to the height and is exactly equal to the width, so nobody bothers explicitly calculating it with circles. But just because a circle's width and height happen to be equal to the circle's diameter, that doesn't mean the circle's width and height are the diameter (or vise-versa). You'd actually be reusing the variable "width" or the variable "height" for an entirely different purpose that just happens to be equal, which wouldn't be good (in this situation, it's a very mild issue - but it's still not a good habit to get into).

There's a more serious, but more subtle issue with re-using the Shape's width, height, x, and y. As I already mentioned, you'd actually be re-purposing either width or height for a different purpose. But how many people here realize you'd also be re-purposing x and y for a different purpose as well?
What is x and y? A coordinate on a Cartesian grid. Right? But in naming it 'x' and 'y' you are actually naming it what it is, instead of what it's for (Which is why it would be better off wrapped in a Point or Coord class). A variable's type should say what the variable is, but the variable's name should describe it's purpose.
What does 'x' mean to a Shape? 'x' does not describe the purpose, it only describes that it is a horizontal location in space. It could serve any purpose - the name isn't describing the purpose.
It's precisely because it's so generically named that you tricked yourself with your Shape class, and also trick future users of your class:
struct Rectangle { int x; int y; };
struct Circle { int x; int y; };

What is the difference between the X and Y of the two above classes?
By convention, the x and y of the Rectangle would be the upper left corner.
By convention, the x and y of the Circle would be the center.

They aren't serving the same purpose! You are repurposing the variables in each subclass to mean something different.
Since x and y really are used to mean a point in space, they should be wrapped together in their own class or struct, and the class' name would then describe the use.
Code using the 'Point' class (like Rectangle or Circle) would then be forced to come up with a new name, and hopefully, that name will describe the actual purpose.
struct Rectangle { Point topLeftCorner; };
struct Circle { Point center; };
Doing this, you can easily see the difference in purpose between Circle's and Rectangle's use of x and y is as different as 'int health' and 'int money' - all they shared in common was what x and y is, but they don't share in common what x and y are for.

It's a very subtle bit of trickery that is easy to fall into. I encourage separating re-occurring usages of variables like that, out from the classes that use them, and into their own class. I also encourage typedefing variables when they don't actually add anything new but are used frequently in a certain way.
int minutes;
What is the 'purpose' of the above variable? Nobody has a clue. It's variable name is actually describing what it is not what it's for.
typedef int Minutes;
Minutes amountPlayed;
Ambiguity removed.

The name of a variable makes sense relative to its context. The context is the class or function that contains it. A Circle doesn't have a x or a y which is an int, and Circle has a 'center', which is a Point in space. A Point has a x and a y which is a int, because 'x' as a variable name now has meaning within the context of a Point.

Diameter is not the same as width. Width is not the same as diameter. Even if they are equal and numerically interchangeable, they are not interchangeable purpose-wise.
The x and y of a Circle and a Rectangle is even farther apart purpose-wise (one's purpose is the center, the other's purpose is the top-left corner).

Regarding the bold bit -- no. You need to investigate the use of composition vs inheritance. Composition is a tool to allow for code re-use (not repeating yourself). Inheritance is a tool to allow for polymorphism. If you're looking for code re-use, you should default to composition, and only use inheritance where necessary.

I would agree on this. The classical orthodox use of inheritance is for specialization. But I think it is common to see inheritance in C++ used (misused?) for extending functionality of a base class, which is really a way of re-use, not polymorphism.

In the Google Go programming language, they went so far as to not support inheritance. So you can only use composition if you want to "inherit" implementation. Polymorphism is solved by implicit fulfillment of an interface, which is a separate declaration. At first, you feel handicapped. But after a while you realize your don't miss the inheritance hierarchies.