private or protected variables?

Question

I am currently reading Code Complete where McConnell strongly encourages making all variables private. Coincedentally I just so happened to be working on a project where I needed to change a private variable.

The class had a private variable (a String) telling it where to load an image from to use in the system chrome. I needed to change this image, I do not know about other languages but as far as I know in Flex/AIR, there is no way to override a private variable.

If it had been declared protected, I could have simply extended the class, and overridden that variable. But since it was private, I had to copy all the code from the class and create a duplicate class with the only difference being that string.

I think the argument is to use private as it makes for looser coupling between super and subclasses, however I had to completely violate DRY to be able to achieve a simple string change, which seems to me as worse.

This makes me think that protected is better than private. However, I want to do things the right best-practices way. So if private is better, I want to understand why.

If the general consensus is that private is better, can someone explain why?

Answer 1

Make variables private for when you know only that the particular class they are defined in will be the only one making use of those variables. However, if you're looking to extend a class, you should be using protected. Private is really so that you are not using variables globally (so you make use of getters and setters in classes) which helps loosen coupling. Protected is perfectly acceptable when extending a class.

In general, I'd say use the access type that most hides the variable from outside classes and/or packages.

Answer 2

In this case, the location of that image used to be a private, implementation-specific feature of the base class. Your new requirements meant that it needed to be able to vary from one derived class to another.

You should keep the member field private, but define a protected virtual property to expose it to derived classes:

private const string _defaultImagePath = @"C:\whatever.bmp";

protected virtual string ImagePath {
    get {return _defaultImagePath;}
}

In the derived class that wants to change it:

private const string _myImagePath = @"C:\other.bmp";
protected override string ImagePath {
    get {return _myImagePath;}
}

You will also want to change the base class so that it uses the property when it needs the image path, instead of using the field. This is the "Encapsulate Field" refactoring.

Answer 3

Everything that is part of the implementation of the class must be private, that makes you have the liberty to change it, if the need arise.

In the example you mentioned, maybe the interface of the class wasn't defined completely because you need to change a private property of the class. If you have control of the code you could make the property protected or add functions to get and set the value.

Answer 4

"Best practice" is itself a subjective term, while there are many conventions that serve to improve one's code, the value of each must be contrasted with the requirements of the problem you are trying to solve.

The declaration of that variable as private should be taken as an expression of the develors intentions for the variable (i.e. internal use), which means you either have to find another extensibility point or duplicate code.

From an API design perspective, one should use private to isolate implementation details from callers and protected to expose things to inheritors

Answer 5

If it had been declared protected, I could have simply extended the class, and overridden that variable. But since it was private, I had to copy all the code from the class and create a duplicate class with the only difference being that string.

And if there had been a setter for the image location, you wouldn't have needed a subclass (or a new class) at all.

And if instead of wanting to change the image, you'd wanted some other minor change, then perhaps making some other API protected/virtual may have helped. Or perhaps not. My only point is that it's hard to look at once nebulous example like this and derive general wisdom from it.

There's a trade-off between how much flexibility a class gives you in terms of its behavior and extensibility, and how much effort it takes to design and test the class (and further interactions among how understandable, well-documented, etc. the class is). These design trade-offs cannot be evaluated in a vacuum, rather they must be evaluated in context. How likely are the various features/behaviors/extensions/changes that a class could possibly support actually going to be? The well-designed class is one that has extensions for common necessary scenarios and lacks unnecessary extensions and APIs. (And for the extensions that are there, there's a choice between simple 'setters' and more complex protected/virtual/subclassing extensibility... the latter is not a trade-off to opt into lightly.)

I don't know that this answers your specific question well (or at all), but the way I read your question, it made me feel like you were implying that "it's better for classes to be swiss-army-knives because extensibility/customizability is good", and I wanted to call out that these 'good' things come with trade-offs. Also, your question seemed to imply that this situation was about 'private' versus 'protected'-and-'subclassing', whereas I think a simple 'setter method' would be a sufficient API change.

Answer 6

Private is, IMO, always best at first, and then make public/protected as the need arises. It's much easier to create a new method than attempt to remove functionality someone was depending on.

In this case, I would think the best approach still would have been to simply have included a setter. Was there any other functionality that your subclass had to provide, or could you have just created a new instance, set the path and be done with it?

Answer 7

Make it private until it needs to be protected. This is an abstraction problem with languages like C#, C++ and Java where protected/private is tied to the implementation. Private members can be safely automatically inlined whereas protected cannot. This is similar to the abstraction problem in C# and C++ with monomorphism and polymorphism needing the virtual keyword.

This essentially makes a fragile base class problem. Defining everything as protected/virtual is the safest in terms of reuse but hinders optimizations. Defining everything as private/non-virtual is the most efficient.

In a language like Eiffel, the compiler automatically detects if functions can be inlined and automatically detects if a feature is polymorphic or monomorphic. http://dev.eiffel.com

Answer 8

When I first got started in OO, I used to make everything public or protected, figuring: "Why should I restrict what other people wanted to do with my code?"

As I got more experience, two things happened:

1. Modern IDEs came along, making it ridiculously easy to change the code.
2. I had to maintain a lot of nasty code that had been extended in ways its designers hadn't intended.

Now my approach is: make it private until you need to make it public (or protected) -- and even then, think hard about whether that's really the right solution.

A good protected method is an intentional extensibility point, put there by the designer. I've found that designing for extensibility is actually kind of hard, and you have to do it -- if you just leave it open and hope, you're asking for maintenance nightmares down the road.

Answer 9

I may be the only one here who agrees with what I think the author is implying. The idea of having to open up a superclass to make changes to an accessor may not be simple and may not always be an option. The private-until-it-needs-to-be-protected method is ludicrous and ultimately means you didn't really have a reason to use private in the first place.

The real reason that you would want to keep a member private is so that an instance of a subclass could not have unexpected access to the members of an instance of a superclass that should remain locked. The unfortunate side effect is the lack of extensibility. The top-voted solution by John Sanders is probably the best practice because it addresses this issue.

However, I've managed to code thousands of classes with frequent use of protected (and almost no private) variables and have never had anything I would call a problem with unexpected usage. It seems that the same can be accomplished by simply choosing not to code in a way where you're messing with other classes protected values.

So in conclusion, I say:

• Keep on coding using protected but remember that this opens up sensitive data to subclasses.
• Enjoy the benefits of easy extensibility, but remember to maintain an 'is-a' relationship between your super- and sub-classes so that the behavior of each stays true to the intension of the method.
• Use the private variable / protected getter technique if you're hardcore or if you need added assurance.

Answer 10

Protected members are part of the API, since derived classes in other packages (i.e., packages outside your organization's control) can see them. As such they represent a commitment. The advice to prefer private amounts to advice to avoid making unnecessary commitments. You can always make a less visible member more visible, but you can't always do the reverse.