The random utterances of David Arno

Why the “private” keyword is the modern day “goto”

evil-privateMost developers these days will tell you that one aspect of object-orientated (OO) best practice is to make member variables private. I contest that they are wrong and that not only is it bad practice, it can force others to have to implement nasty hacks to get around the use of the private keyword.

If you are old enough to remember programming in the pre 1990s, you’ll likely have used the keyword “goto” in your code at some point and the longer you have been programming, the more you will have used it. If you came to programming after that, you’ll (hopefully) have been told from day one not to use it. You may even use a a language that doesn’t support it.

The “goto” statement took a long time to die, but it has gone from being a fundamental programming concept to a programming pariah used by only the most misguided of developers (and possibly a few folk working on extreme edge-case applications.) This highlights the fact that software engineering/ craftsmanship is an evolving field, where what constitutes best practice can change over time. Other previously considered best practices have suffered a similar fate to the goto keyword, and I’ll cover some of those later. Others though seem to either resist challenge, or just never get properly challenged. I wish to address one of those here: the “private” keyword.

Why do folk use private? There are two aspects of OO that at first glance seem to encourage it: encapsulation and inheritance:

Encapsulation
“Data hiding”, or encapsulation is key to OO. If I have a list class, I allow other classes to interact with the list, but I hide the list’s implementation away. Any developer using my List class should not need to worry about how the list is stored, instead I should provide a set of List-related methods, eg Split, AddToHead for manipulating it.

Inheritance
As “Uncle Bob” puts it: “The Open-Closed Principle … says that objects should be open for extension but closed for modification. In other words, you should be able to change what a module does without changing the module”. This principle was regarded as best practice in the early days of OO.

The combined requirements of data hiding and preventing modification through inheritance inevitably gave rise to the private keyword for hiding member variables (fields) away. The thinking being that fields themselves should be private and that public methods should be provided for accessing those fields.

Many younger developers may not aware of this, but this “open for extension but closed for modification” mantra led to some pretty daft ideas. One such was that if I had some arbitrary code:

Class X
  Method Div(p1, p2)
    Return p1/p2
  End Method
End Class

when it was noticed that Div(n, 0) results in an a divide by zero error, method Div would not be fixed as that would be modification of the API and so could impact existing code. Instead one would extend, eg:

Class X
  # Don't use this method in new code as it results in an error.
  # Use Div2 instead
  Method Div(p1, p2)
    Return p1/p2
  End Method

  Method Div2(p1, p2)
    If p2 <> 0 Then
      Return p1/p2
    Else
      Return NotANumber
    End If
  End Method
End Class

In the early 90s, I actually tried to argue for the adoption of this practice in the company I worked for at the time. With hindsight I am very pleased that I was voted down by my more pragmatic colleagues. These days, with the great ideas of unit tests and refactoring gaining in popularity, the mantra of “open for extension but closed for modification” has gone from a mere dubious idea to a downright debunked one. Modifying through refactoring is now positively encouraged.

There is another twist in the tale of why “private” is so popular and that is due to properties (getters and setters if you prefer, though to my mind these should refer only to the weird accesor methods that Java has instead of proper properties). Because one can wrap a private field in a public read/ write property, there seems to have developed a consensus that one should do so always without exception. This is a bad state of affairs as it results in ridiculous stuff like:

public class ComplexNumberVO
{
  private double _real;
  private double _imaginary;
 
  public double real
  {
    get { return real; }
    set { _real = value; }
  }
 
  public double imaginary
  {
    get { return _imaginary; }
    set { _imaginary = value;  }
  }
}

Why do I claim this is a ridiculous class? Two reasons. First, there is only pseudo encapsulation occurring with the class, as the the two properties fully expose the underlying data. They serve no purpose whatsoever other than to add complexity to the class (thus increasing the chances of bugs) whilst fulfilling some grossly misguided belief that best practice is being followed. The other is that it is a classic attempt at writing a value object (VO) by someone who doesn’t know what a VO is. A key feature of a VO is that it should be immutable: its a value, not a reference. Shame on you if you didn’t spot that! ;)

Before we go any further, let me make one thing clear: the well thought out use of public properties and hidden fields is good practice as it hides the implementation. What I’m suggesting is that blindly using them in a simple full-mutable record-style (or tuple-style if you prefer) class is bad practice. We can rewrite the above class to make it a proper VO:

public class ComplexNumber
{
  protected double _real;
  protected double _imaginary;
 
  public ComplexNumberVO(double real, double imaginary)
  {
    _real = real;
    _imaginary = imaginary;
  }
 
  public double real
  {
    get { return real; }
  }
 
  public double imaginary
  {
    get { return _imaginary; }
  }
}

You may notice that I’ve not only made the class a true VO, I’ve switched the fields from private to protected. This brings us to the main topic: why using protected rather than private for fields should be regarded as good practice and the use of the private keyword should be avoided at all costs.

As previously mentioned, one reason people use private rather than protected stems from the outdated idea of “extend, do not modify”, but there is more to it than that. There is also a belief that developers somehow need protecting from side effects within the base class when extending it and from needing to know about the base class’ inner workings. Many such folk seem to view public and protected as somehow equivalent. They are not. Protected respects encapsulation; public doesn’t. Finally, people just use private because that is what other folk, and many automatic tools, do.

Regarding the second point, I personally find this attitude both patronising and symptomatic of a lazy attitude to writing classes. Its patronising in that the base class’ developer doesn’t trust me. It’s lazy because documenting a class properly for the benefit of anyone wanting to extend it takes effort. It’s much easier to just mark everything private and hide it away. This attitude is especially annoying when used in frameworks or libraries for which the source is available. I can see those useful fields and support methods sitting there tempting me, but I can’t access them.

I’m sure I’m not the only one who has had to resort to cutting and pasting the code for a set of private members out of a base class into a sub class in order to modify some subtle aspect of the base class’ behaviour simply because the original developer used the private keyword. If they’d used protected, then the issue would never have arisen. If you have ever done more than simple work with the Flex SDK, then you’ll likely know exactly what I mean.

Even when the source isn’t available, debuggers taunt the developer with details of private members beyond our reach. This, coupled with the over-zealous use of the private keyword, has even spawned an entire micro-industry in hacks to the Flash event classes to work around its perceived shortcomings.

I found an article by the previously mentioned software engineer, “Uncle Bob”, which argues the opposite of this article. It is worth addressing the points he raises in order to explain why those points are misguided in my view, which should reinforce why I’m so anti the “private” keyword.

“How do you protect a module from the forces that would try to modify it?”
As previously discussed, inheritance is all about modification (what’s the override keyword for, if not for providing alternate behaviour for a method for example?) If you want to protect a class from modification, make it immutable and final. If you don’t make it immutable and final, accept that someone developing a sub class is doing so because they want to modify your class’ behaviour! So please make it simple for them to do so.

“If a variable is not private, then it is open to be used in a way that the module that owns that variable does not intend … For example, given a variable v used by a module m, such that v should never be negative. If you make v … protected someone could set it to a negative number breaking the code in m…”
Write your class. Document how it works. Document that v should not be negative and explain why. Then step away and leave sub class developers to make up their own minds. If I chose to make v negative, then I have to deal with the consequences. Don’t treat me as a child and try to prevent me doing so. If course if v is to be accessible outside of m‘s inheritance tree, put a public property in place to prevent outside agencies making it negative. Again: public and protected are not equivalent. Protected respects encapsulation; public doesn’t.

“Privacy does not preclude extensibility. You can create public or protected accessor methods that: 1) provide extenders access to certain variables, and 2) ensure that the extenders don’t use the variable in an unintended way.”
Let’s rewrite that to reflect modern software development techniques and what ought to be best practice. Encapsulation does not preclude access to an object’s protected values. You can create public properties that provide external functionality access to those values whilst ensuring that the external functionality cannot modify those values in an unintended way.

Conclusion
To wrap up, here is a list of what I’d regard as best practice with regard to what’s been discussed here:

  1. Avoid using the “private” keyword. If possible, never use it. Pretend it doesn’t exist.
  2. Follow some sensible rules of encapsulation:
    1. Is the class as simple record/ tuple-style class (a DTO) with no methods for causing side effects to the fields? If so, make the fields public.
    2. If not, does the field contain data that needs to be exposed to external functionality? If yes then make it protected and wrap it in a public property only if changing its value has side effects, it needs validating or it is a read-only value. If it doesn’t need validating, has no side effects and doesn’t need to be read-only, leave it public.
    3. If the field should not be exposed to external functionality then make it protected.
  3. Properly document your class so that developers can easily extend it and can understand the consequences of modifying the base class’ behaviour.

Remember, unless you unlucky enough to be using Java or another archaic language with no proper property support, you can always refactor your class to replace a public field with a protected one with a public accessor property if requirements change. Refactoring is the key: do not design your class with future expectations in mind. Design it to meet only your current requirements and write unit tests to test that functionality. Then in future you can refactor to meet new requirements (which probably won’t be what you were expecting) confident in the knowledge that your unit tests will pick up any breakdown in existing functionality. If enough developers adopted these modern development best practices, we just might be able to consign the private keyword to software engineering history, which is where it belongs.

UPDATE:
I have written a new – shorter! – article that demonstrates the unit-testing benefits of using protected rather than private. See ‘Real world use-case of “use protected, not private”


Share This Post...
62 comments so far, click here to read them or add another

62 Comments so far

  1. bezoarqueen July 23rd, 2010 09:00

    The author’s argument is exceptionally weak. And when arguing against what the language designer explicitly states as best practices, you should have a solid argument. I will continue leaving my member variables private unless there’s some solid reason not to.

  2. almbfsek July 23rd, 2010 09:12

    I too find the author’s argument weak.

    Though, as a JS programmer I don’t find the lack of ‘private’ members that troubling. If I’m writing a public interface I can always enforce the use of appropriate methods/properties via documentation.

  3. deafbybeheading July 23rd, 2010 09:38

    tldr: “I may want to use your class in ways you didn’t anticipate; so just let me get at the internals.”

    The problem with that approach is that it’s more work for the original developer (documenting internal state and invariants), combined with less flexibility to refactor (a documented internal API is still an API that, for moderately-widely-used libraries, must remain fairly stable).

    Really, most of the problems he alludes to would be better solved with SRP, interface-based programming, and composition.

    Asking for all private fields to be protected instead is tricky. On the one hand, it’s “I need to use your class in ways you didn’t anticipate.” On the other, it’s “Hey, don’t worry about designing anything properly. It’s cool–I have access to the internals, so I can hack around it it.”

    I think the latter is certainly more pragmatic, but it encourages sloppy design.

  4. physicsnick July 23rd, 2010 09:41

    This is crazy. He complains that a complex number class is bad because it exposes its internal data using properties. What is his “best practices” solution?

    public class ComplexNumber
    {
    protected double _real;
    protected double _imaginary;

    public ComplexNumberVO(double real, double imaginary)
    {
    _real = real;
    _imaginary = imaginary;
    }

    public double real
    {
    get { return real; }
    }

    public double imaginary
    {
    get { return _imaginary; }
    }
    }

    No joke: that is the actual code example on his site. getters only, because it should be immutable; but protected instead of private, in case a potential sub-subclass wants to mutate it?

    I may not be terribly “hip” with the latest managed enterprise languages, but what exactly have you gained by making an inheritance tree and properties instead of just this:

    struct complex {double real, imaginary;};

  5. neil manell July 23rd, 2010 10:20

    You don’t mention the effect of private fields on unit testing…

  6. Mike July 23rd, 2010 12:03

    Wow, just wow.

    I have read a lot of misguided information touted as ‘the new best practice’ – but throwing out the private keyword just because it can make inheritance inconvenient is ridiculous.

    Ever programmed in C++?
    Know what it’s like to find out that some other part of the code has a direct pointer to one of your member variables because it was public?

    The problem isn’t private – the problem is the atrocious over-emphasis on inheritance with the Object oriented paradigm. Most developers have no idea how a given class will be used or extended into the future. If you have a new requirement that warrants a subclass, chances are you’ll need to refactor the base class to accommodate it anyway.

    Instead of advocating that inheritance should be easy with protected and public members, we should recognize that composition trumps inheritance just about every time.

    An inheritance relationship is the strongest form of coupling in OO. Instead of inheriting from a class to create a variation, refactor that class to allow it to be configured to behave the way you need it to.

    If we need to kill a keyword, I vote to get rid of protected.
    It encourages strongly coupled inheritance and rigid designs that impossible to grow organically.

  7. David Arno July 23rd, 2010 12:37

    @bezoarqueen, would you care to expand on why you feel my argument is weak?

    @almbfsek, you are right. The lack of encapsulation (other than via some horrible bodges) is the least of your problems with javaScript.

    @deafbybeheading. You have misunderstood what I was arguing. The core reason for replacing private with protected is to improve a class’ design and to avoid the need to hack.

  8. David Arno July 23rd, 2010 12:38

    @neil,

    You are right, replacing private with protected has huge benefits when it comes to unit testing. I planned on dedicating a separate post to that topic though :)

  9. Alex July 23rd, 2010 12:49

    @physicsnick: When I started reading the paragraph about how the VO class was “ridiculous,” in my mind I expected the rewrite to be exactly the struct you mention, not the lukewarm getter-only direction the author went. If I were to write this article I would go in the direction of advising for small stateless static functions, and data storage using immutable structs.

  10. David Arno July 23rd, 2010 12:52

    @physicsnick,

    Your struct example is mutable, has no encapsulation and is not extensible. My immutable class example with proper encapsulation, and support for enhancement through inheritance trumps it in every regard.

    You express outrage at my code example. This is typical of the reaction I get from developers who blindly follow the “make it private” principle. Let me give you an example of why it is actually excellent design.

    Imagine I have a system that generates a series of graphs based on a changing complex number. I could use a round-robin approach of constantly calling a set of functions, passing in a new complex number each time. This would work, but it’s incredibly clunky and doesn’t scale well. Alternatively, I could create a sub-class of ComplexNumber that uses a callback to expose its fields to just the main controlling class. I then use dependency injection to give all the graph drawers access to a reference to that value. The graph drawers can then run independently (say in different threads) and only the controlling class gets to change the value contained by that reference. This is a far neater. more robust and more scalable solution.

  11. David Arno July 23rd, 2010 12:59

    @Mike,

    I have had the misfortune of having to code in C++, yes. Thankfully these days I’ve moved on to modern managed languages that avoid the nightmare of pointers.

    I do not agree that composition trumps inheritance every time. Each has its advantages and disadvantages (one must use inheritance if polymorphism is required for example).

    I’m not dismissing composition, I just regard it as a solution to one problem and inheritance as a solution to another. Both are useful OO tools.

  12. Amy July 23rd, 2010 13:00

    I think anyone who has ever worked with Flex in any depth will agree with David. If you’ve ever tried to extend AxisRenderer, you quickly realize that the engineers at what was then Macromedia made a lot of stuff private that essentially makes it impossible to extend the class.

    In theory, you may be right that all developers ought to write everything absolutely perefectly to make it extensible with most things private. HOWEVER, that implies that all of these theoretical developers are much better than the engineers at Macromedia, which is now part of Adobe, one of the largest software companies in the world.

    Guess what? I’d be willing to bet that at least 85% of engineers in the world are not even as good as those engineers. So, given that most of us are not as perfect as we’d like to be, all we can do is make sure our imperfection doesn’t completely screw over the next guy.

  13. David Arno July 23rd, 2010 13:01

    @Alex,

    I avoided C# structs as they are language specific and I was trying to be as generalised as possible with the article. @physicsnick’s syntax suggested he was referring to C-style structs, not C# ones. They clearly have no advantages (beyond being easier to hack ;)) over an immutable class.

  14. David Arno July 23rd, 2010 13:03

    @Amy,

    I think you summed up what I was trying to say brilliantly: “…given that most of us are not as perfect as we’d like to be, all we can do is make sure our imperfection doesn’t completely screw over the next guy” :)

  15. not-your-mama July 23rd, 2010 13:37

    Have you ever programmed in C? goto is not a pariah, and using it is not reserved for misguided nimrods as you allude. Quite the contrary, it is the canonical (and correct) way to handle many cases, especially error handling code.

  16. Stephan Schmidt July 23rd, 2010 14:09

    “Modifying through refactoring is now positively encouraged.”

    It’s funny how many people use the word refactoring and mean exact the opposite of how it’s defined.

    Wikipedia says:

    “Code refactoring is the process of changing a computer program’s source code without modifying its external functional behavior in order to improve some of the nonfunctional attributes of the software.”

    while you argue refactoring is used to change behavious (removing the p1/p2 exception)

    Best
    Stephan
    http://codemonkeyism.com

  17. Tarski July 23rd, 2010 14:10

    Immutable classes should be final (not extendable). The whole argument for making fields protected and not private seems to rely on subclassing other people’s APIs – a really bad idea, because overriding can break the behaviour of other methods, and as the APIs change they can break your client subclasses.

  18. David Arno July 23rd, 2010 14:20

    @not-your-mama,

    I knew someone would leap to the defence of “goto” at some point. Thanks for making me laugh :)

  19. anonymous coward July 23rd, 2010 14:20

    Wow. Is this random utterance or what?

    goto was never a pariah. Despite the heated allegations from uptight dogmatic “authorities”, goto is still useful in non-trivial, non-edge-case projects (maybe mention Linux and BSD kernels as such). In fact, it’s useful to anyone who can consider its benefits.

    Same with private. Just because you aren’t able to understand some simple key OO concepts and guidelines, doesn’t make the rest of us blind followers of anything in particular.

    Actually, here’s an idea: go seek some of our more pragmatic colleagues down in nVidia, who will tell you how refactoring has led them to the great 196.75 driver fiasco.

    Cheers.

  20. Charles Nugent July 23rd, 2010 14:21

    Most cogent response to this argument so far can be found here:

    http://news.ycombinator.com/item?id=1540925

    “The author seems to misunderstand the open-closed principle, and his first example confuses bug-fixing with API modification. The open-closed principle is designed to promote API stability and reduce errors caused by API contract changes that cascade to its users. A class should expose an interface that allows for such contract changes to be encapsulated in subclasses. This is not a “debunked” or “outdated” idea, especially if you are designing APIs for public consumption.”

  21. David Arno July 23rd, 2010 14:24

    @Tarski,

    Fair point regarding making immutable classes final. It certainly makes for a clean solution.

    I don’t agree that subclasses APIs is a bad idea though. I guess it depends on the API, but in the case of visual components, it is normal practice when one wishes to modify their behaviour in some way.

  22. David Arno July 23rd, 2010 14:34

    @Charles,

    Regarding the comments you link to, “btn” is the one that is confused over “open-closed principle”. I’d suggest you read the comment by “edanm” directly below in which he gives a real-world example of the point I was making. Without unit tests to make the developer feel secure in fixing bugs, it is often pragmatic to leave the bug well alone and create a new version of the functionality without the bug.

  23. Zak July 23rd, 2010 14:41

    It’s lame when you delete comments that disagree with you!

  24. chris July 23rd, 2010 14:50

    You should always use the best tool for the job, sometimes that’s private, sometimes that’s protected, and sometimes that’s public. Just because you have some beef with programmers who don’t understand that, you want to abolish a perfectly useful tool, proving you’re no better than they are. Worse, you use the weakest examples possible. Anyone who would extend a class with an obvious flaw instead of fixing it, specifically when that change is only internal and wouldn’t affect the methods, shouldn’t be allowed to program ever again.

  25. David Arno July 23rd, 2010 14:59

    @Zak,

    I completely agree that it would indeed be lame to delete a comment that simply disagreed with me. However it’s very sensible to delete puerile “your dumb” comments as it helps keep the tone of the criticism professional. I’m sure you’ll agree with that.

  26. Mark Cidade July 23rd, 2010 15:07

    One of the advantages of making members private is that it gives the developer the freedom to change the implementation without breaking anyone’s code—it’s a guarantee that any compliant calling code won’t have any direct references to the members so you can do whatever you want with them.

    The problem with making members protected is that they then become part of the public API—if you try to change them afterwards, you might break someone’s code.

    Your argument is better suited towards making code open-source, not why the implementation should always be exposed somehow.

  27. Zak July 23rd, 2010 15:26

    I didn’t say “you’re dumb,” or as you stated it, “your dumb” (hmm, OK, I’ll let that one go because it’s just too easy). I said “This is the dumbest article I’ve read all week.”

    I wasn’t attacking you personally. You seem like a bright fellow. But your recommendation to eliminate the private keyword shows a lack of understanding about the purpose and merit of encapsulation and the problems caused by unintended side effects.

    There’s a reason that consumers cannot open an iPhone and play with all its internal parts. It’s the same reason developers may want to keep the internals of their classes private.

    But I’ll have to give you credit for sticking your neck out like this.

  28. Jed Smith July 23rd, 2010 15:42

    David,

    Although you mock those who defend goto, you’ve put it at the core of your argument by comparing private to it. Whatever your opinions on private members – and others have done a fine enough job highlighting your error there – your underlying comparison is flawed.

    In at least one common case, goto is the only way out of a situation. You mentioned that you have done some work in C++, so I’d expect that you’d know what I’m talking about. I, too, have heard from lower-level CS classes (part of my reasoning for avoiding said curricula) that goto is “evil” and should be “avoided”. How then, in its absence, do you propose getting out of this?

    for(int y = 0; y < rows; y++) {
    for(int x = 0; x < columns; x++) {
    for(int i = 0; i < members[x][y]; i++) {
    if(members[x][y][i] == WhatImAfter)
    search = members[x][y][i];
    /* Found my member! How do I jump all the way out of the loops? */

    Although you might be quick to call such a table scan pure naivete, there are many fields where such a table scan is *regular* in the program's tight loop. Think game engines for mapping – an example of this can be found in Metropolis, the open-source SimCity, where a full map scan is done regularly enough to be a macro.

    Once I've found the element, what do I do? In your world, this:

    for(int y = 0; y < rows; y++) {
    for(int x = 0; x < columns; x++) {
    for(int i = 0; i < members[x][y]; i++) {
    if(members[x][y][i] == WhatImAfter) {
    search = members[x][y][i];
    foundIt = true;
    break;
    }
    }
    if(foundIt) break;
    }
    if(foundIt) break;
    }

    So now, in this potentially tight loop where milliseconds might be crucial, you've asked me to add a boolean compare to, at the worst case, every cell up to the one containing the member I'm looking for. All in the name of just avoiding:

    for(int y = 0; y < rows; y++) {
    for(int x = 0; x < columns; x++) {
    for(int i = 0; i < members[x][y]; i++) {
    if(members[x][y][i] == WhatImAfter) {
    search = members[x][y][i];
    goto FoundIt;
    }
    }
    }
    }

    FoundIt:
    printf("Hooray!");

    In your argument against private members, you condemned (wisely) those who blindly follow an ideal without understanding its purpose. However, by condemning goto so harshly, you have demonstrated the exact same flaw – someone once told you that goto was bad, so you've spent a large part of your career working around it.

    Are there other ways to do the table scan? Perhaps. Is it worth rewriting the table scan to avoid a situation where a goto is necessary? Absolutely not. Simply based on your ignorance with goto, I think your argument against private members should be a bit better-prepared. Others have made that point for me, though.

  29. David Arno July 23rd, 2010 15:42

    @Stephan,

    I am guilty as charged for being sloppy in my use of the word “refactor” :)

  30. Newtopian July 23rd, 2010 15:54

    I always had a reserve about private but so far it was more touchy feeling than based on factual arguments.

    But the same arguments brought forth by the author and some people commenting herein can be used verbatim against them.

    The assertion that most programmers are not the brightest light bulb is sadly true, to a point.

    therefore the argument where the author of a library, API or other piece of code cannot anticipate all possible uses of his brainchild. As such he should allow users of the code to tinker with it more intimately to extend or alter it so that it is suitable for such unforeseen use.

    But to ban private altogether would be a mistake. Using the same arguments in reverse I would say to trust my insight and intelligence when I say that this part is really too intimate to be tinkered with by strangers. Many practical examples come to mind where the implementation of a functionality should remain private and for this languages like java probably should go even further in defining accessors restrictions based on package hierarchy in addition to class hierarchies. This would come very handy when building an API where implementation classes must be made public but should really not be accessible outside of the API. But this is another debate.

    My rule of thumb here would, instead of “Never, Ever stay away from this cancer, use Private” sound more like :
    Make it private if you have a very good reason to do so, otherwise make it protected.

    This gets rid of the systematic make it private that is causing so much pain by hiding too much but still allows designers of libraries to rely on a level of abstraction where re-factors can be done to the implementation with a good level of confidence that client code will not break from the changes.

  31. paul_eye July 23rd, 2010 16:00

    Agree with Chris in that we should use the best tool for the job. So many of us are quick to find an answer to a solution, and then praise it as the next big thing.

    To me, removing private is the same dogmatic approach as developers who create getters/setters for every property. When developing there is no one right solution, you must look at the problem, and apply common sense.

    David makes some good points in the article, but it does not convince me that I should abolish private.

    Once again, depends on what I am writing, and what my solution is. Simple DTO, i’m using public properties. There is no api to this structure, I don’t need to hide anything.. use at your own risk.

    Designing a class that is really intending to be configurable, i favor composition over inheritance, and make sure you can inject your own objects to modify the behavior.

    Those objects you inject however, might need to either implement the same interface OR extend a base class. If they are extending a base class, I will need to expose some of my internals to allow this modification.

    We dont need a new religion.. or any for that matter :) Let’s use common sense, and learn from our mistakes.

  32. Jed Smith July 23rd, 2010 16:03

    Oh good, you know what I’m talking about: http://www.davidarno.org/c-howtos/goto-evil-incarnate-or-handy-keyword/

    However, that article is full of the same overbearing statements I found issue with here. “Ideally, you should never use goto” is a pile. Additionally, breaking out of a deep nest is not considered a “special-case” to that “rule”, it’s the damn keyword’s purpose.

  33. David Arno July 23rd, 2010 16:03

    @Jed,

    I used a similar argument to yours when recently discussing the merits of using break to label (which C++ lacks). I argued that break to label was useful for that very reason. The counter to my argument was that if I found myself using break to label, I’d written bad code.

    So in your case, you can avoid the need for a goto by pulling the code you describe out into a separate method. That way the code might become:

    whatever method(rows, columns, members, WhatImAfter) {
      for(int y = 0; y < rows; y++) {
        for(int x = 0; x < columns; x++) {
          for(int i = 0; i < members[x][y]; i++) {
            if(members[x][y][i] == WhatImAfter) {
              return members[x][y][i];
            }
          }
        }
      }
    
      return NOT_FOUND;
    }
  34. David Arno July 23rd, 2010 16:07

    @Zak,

    Fair point. I apologise for pulling your original comment therefore.

    You still seem to be lumping protected and public together as somehow breaking encapsulation. The protected keyword does not break a class’ encapsulation.

  35. Jed Smith July 23rd, 2010 16:09

    Again, just to avoid goto, I should refactor my tight loops into their own function? In some cases when these table scans are done multiple times per second, you now want to add an expensive function call just to get out of using goto?

    That’s not a solution, that is demonstrating what I discussed in my comment. Blind belief, and avoiding a logical solution.

    It also shows that you have poor judgment about when to craft a function. What if I have different forms of this table scan? Now I need to prep my arrays before I pass them into this function? Or, do I have six different functions for the different table scans I do on my data structure?

    You’ve lost touch with real, in-the-trenches programming if you honestly believe that goto must be avoided at all costs. As someone on HN said, you disagree with the way some less-experienced programmers operate so you want to change the game for the rest of us.

  36. Daniel July 23rd, 2010 16:10

    By definition, refactoring is changing what a “module” does without changing its contract. If you change the contract, it’s not refactoring.

  37. David Arno July 23rd, 2010 16:33

    @Jed,

    We are wandering way off topic here, but you touch on some interesting points.

    Pulling such a loop out into a separate function is clearly good practice. You should always strive to keep methods as small as possible. A good rule of thumb is when describing what a method does in natural language. do you use the word “and”? If so, refactor it into two smaller methods.

    Modern compilers will in-line functions for you whenever they can, so there will be no increase in overheads.

    Remember: write good code and let the compiler turn it into optimal code. Don’t try and do the compiler’s job for it.

  38. Jedediah Smith July 23rd, 2010 16:45

    @Jed

    Wow, I’ve only met one or two other Jeds in my whole life, and now another Jed Smith who is a programmer? And I wasn’t googling my name or anything, just reading the article. Damn freaky. Email me.

    Sorry for the interruption folks, this is just ridiculously unlikely.

  39. /b July 23rd, 2010 16:49

    I would argue that you have the trust thing backwards. Rather than think “when a library developer makes a member private, he is saying he does not trust the developers that use his library,” I would think “when a library developer makes a member private, he is saying that other developers cannot trust him, as he might make changes to that part of his implementation that will unexpectedly break their code.”

    When you argue that blindly making everything private is the wrong way to go, I agree with you, and am not surprised that you have encountered situations where it has made your life difficult. But you then formulate the opposite rule: never use private at all. The real problem is blindly doing anything.

  40. Rich July 23rd, 2010 16:49

    Most programming techniques when applied without common sense are like a genius without direction, they are both clever and useless at the same time.
    There are times when using private will benefit and there are times when using private will falter, the mark of a truly good developer is knowing which time is which and apply the relevant technique.
    To blindly follow what is commonly regarded as “best practice” is to deny the one thing that seems to separate us from other animals our intelligence.

  41. Zak July 23rd, 2010 17:16

    The simplest definition I’ve seen for encapsulation is “information hiding.” OO encapsulation hides complexity, shields clients from implementation changes, and reduces unwanted side effects.

    Encapsulation doesn’t apply just to external classes, but also to derived classes, and the concept is just as valid. I don’t understand how you can advocate for ‘protected’ but rail against ‘private’, given they both provide levels of encapsulation.

    I can’t wait to read your article about the ‘sealed’ keyword. ;-)

  42. Vorlath July 23rd, 2010 17:57

    Great article! I’ve been saying for years that anyone that uses the ‘private’ keyword should be banned from programming for life. I get pretty much the same response as you’re getting here. Some of the comments are funny. Why not fix the internals of a flawed class instead of extending it? Why indeed!

  43. Sam July 23rd, 2010 20:08

    Div2 should return Div if the denominator is non-zero. That way you retain use of the (perhaps otherwise well-tested) Div function, and fix the bug by decorating it with a guard clause.

    But how is fixing the bug in Div a breaking API change? The OCP applies when modifying the behavior of a module. The only way fixing Div could be considered modifying its behavior is if clients relied on its ability to throw a DivideByZeroException.

    But then the so-called bug is actually a specification of that method’s behavior. From the point of view of its API, it is supposed to work that way. Then it naturally makes sense to introduce a second method that alters the original method’s specified behavior by decorating it.

    I contend that the OCP does not apply when a module does not pass its specifications, for in a sense, the module has not been fully written yet. Same with writing a class skeleton without any methods in it. Just because you hit ctrl-S to save the class, does not mean that it now must adhere to OCP.

    Even if a module has shipped, I would argue that it is still ok to modify it, if it isn’t behaving as specified. Only consumers that rely on its not behaving as specified will get burned.

    (Of course, a module should never ship if it doesn’t even behave as specified.)

    Summary:
    – Modules not behaving as originally specified should be changed.
    – Modules behaving as originally specified should not be changed.
    – Modules with additional behavior that is unspecified but passes all original specifications should also not be changed.

    Whether or not Div should be modified depends on whether or not a specification was written for when the denominator is 0.

  44. ct July 24th, 2010 00:51

    The original assumption is also false… goto is not dead, it is not used in ‘edge cases’, and it’s use is not by misguided developers.

    The “goto” statement took a long time to die, but it has gone from being a fundamental programming concept to a programming pariah used by only the most misguided of developers (and possibly a few folk working on extreme edge-case applications.)

    The use of goto is not always misguided. Consider C where common clean up code is required. I wouldn’t consider an C developer working on an ‘extreme edge case’.

    foo* use_goto(int v) {
    struct foo *f = NULL;

    f = malloc(sizeof(struct foo));
    if (!f) {
    goto fail;
    }

    if (!init_foo(f)) {
    goto_fail;
    }

    return f;
    fail:
    if (!f) {
    free(f);
    }
    return NULL;
    }

  45. David Arno July 24th, 2010 13:44

    @ct,

    Someone today using simple C, rather than C++ with its try, catch features is an edge case themselves.

    I’ll leave the goto support fringe with this thought: http://xkcd.com/292/

  46. Rich July 27th, 2010 09:25

    fantastic, the article made me laugh but no where near as much as the comments that have been left. Everyone is so passionate about private, god alone knows how many comments would be left on a piece about the correct formatting of braces or allowing tabs in code. I love these pointless arguments designed so that I can fritter away a few moments whilst jobs run.

    1. If a class is designed for extension then you probably will find yourself using protected, however there is normally little need to make classes extensible as a key design you would normally enforce this via interfaces or whatever your chosen language equivalent is.

    2. If its private then I as the designer really don’t want you to have access to it.

    3. If you find your classes that weren’t intended for extension need to be extended then, sorry to be the one to tell you this but, find a new carer this one doesn’t suit you as you are basically making the wrong decision from the very first design steps.

    4. goto is “usually” a bad idea, it is almost certainly a bad idea in higher languages as there are more versatile alternatives however the closer to the machine you get the more likely you are to find the need to use them as they are how the majority (if not all) processors make decisions that branch changing the instruction pointer and all that jazz. I guess it’s for this reason the one person that jumps to its aid is using C.

    5. I think this article is borne out of the fact that action script is pretty dire when it comes to variable scoping so you can be forgiven for assuming that private is such an arse in other languages if you’ve had no experience in any others.

    6. Any method/paradigm/best practice when applied dogmatically without thought or reason to every circumstance is going to have its fair share of failures, we surely are intelligent enough to know when those times are or we’re managers in the making :)

  47. David Arno July 27th, 2010 12:33

    @Rich,

    1. If a class isn’t designed to be extended, make it final, sealed or whatever the keyword might be in your language of choice. If it’s not final, then it’s designed to be extended…

    2. As a class designer, you will write it once. Users of your class (ie other developers) may use it thousands, even millions of times. The onus is therefore on you to make their lives as simple as possible, not your own. It’s not about what you want, it’s about thinking about what your users will want.

    3. I completely agree: always design your class with extension in mind.

    4. Apparently from K&R 2nd edition, page 65: “C provides the infinitely-abusable goto statement, and labels to branch to. Formally, the goto is never necessary, and in practice it is almost always easy to write code without it. We have not used goto in this book.”

    The C language authors themselves recognised that even C did not need a goto statement. Just don’t use it. You’ll be a better programmer for it.

    5. This is completely incorrect. Variable scoping in AS3 is very similar to Java. Whilst not as good as C# (which implements “internal” in a really clever way), to call it dire is to show ignorance of AS3. Private tends to be the same in most languages and is equally a bad idea in all of them.

    6. If you are new to programming, best practice should be strictly followed. As you get more experienced, best practice should be questioned. If a circumstance seems to require a circumvention of best practice, try to convince colleagues etc. If they are convinced, ignore the best practice or change it. If they don’t agree, accept your implementation, rather than the best practice, is what is at fault and change it.

    Basically use best practice pragmatically, not dogmatically.

  48. Rich July 27th, 2010 14:22

    your answers to 1 + 3 contradict each other

  49. Rich July 27th, 2010 14:24

    4. your getting caught up on the wrong point, who gives a tiny rats arse about what language you choose the instruction set for most if not all processors have goto/branch in them from the good ol’ 6502 days to the arm7 to the x86 e.t.c.
    Stop trying to twist the words to suit your argument

  50. Rich July 27th, 2010 14:25

    5. I asked the opinion of someone you hold in high regard on this point before commenting on AS3, am I to believe the love is lost now?

  51. Rich July 27th, 2010 14:26

    6. thanks for re-iterating my point, but why?

  52. Rich July 27th, 2010 14:29

    again on point 3. to make something extensible does not mean to make all variables accessible to the extending class by using protected this is “sometimes” the only way to do so in an intelligible manner but by no means is or should this be the only way to design something with extension in mind.

  53. Skybuck August 9th, 2010 09:41

    The last few years I have also come to the conclusion that private survives little use… and is more cumbersome… fields might as well be protected and offer the possibility of more flexibility in derived classes.

  54. F.D.Castel August 12th, 2010 12:42

    David,

    Excellent article! Just one comment: everything you said about ‘private’ shouldn’t also stand for ‘final’?

    I mean: if you are really really absolutely positively sure that your class should be ‘final’, why not just document it as this and let the developers ‘deal with the consequences’ if they wish to inherit from it?

  55. Balint November 12th, 2010 12:26

    I think two worlds are clashing here: the library developers and the library users. The first want ‘private’ to have freedom to modify their classes without breaking user’s code; the other want freedom to modify the library without touching the library itself.

    I feel the first is the less reason. Modifications are extensions in most cases, and often you can put the modification into a derived class, where you are free to define new private members.

    After I was forced to use ‘#define private protected’ a couple of times, I adopted two simple policies:

    1. I put data members into ‘protected’ instead of ‘private’ if I have no good reason to do otherwise. I use ‘private’ only if I am absolutely sure the connected functionality *must* be handled in this class. The typical guidelines say that data members are best in ‘private’; I do not think so.

    2. never put functions in private. If a function in ‘protected’ is using private members, it is going to be ‘semantically private’ anyway (meaning you cannot put the same code in a derived class), but I let people who are deriving from my class to replace the function or use it if they want. It is their mess.

  56. LSaridina November 13th, 2010 18:34

    When I saw this post title, I said to myself, ‘Hey, this man is throwing the one rock to make the avalanche!’. And the avalanche is keep rollin’ until now.
    Long time ago, when there are no AS3, I believe that any inaccessible part of a class should be labeled as a private member. And after I dive into big projects using AS3 I keep swearing library creators as I had to made a turn around to solve some of possibly simple problems. Sometimes it’s just because I can’t name my new method with the built in method that inaccessible. Sometimes I just need to made a little change to the existing ones.
    All in all, +1 for modifiable private methods, keep the private still in private, but let me modify it if I really need to do. In other words : Use protected for useful methods! (That pretty the same, but I hope you understand what I meant to say)

  57. Socob January 25th, 2011 21:56

    Hey there David,
    I’d like to ask you a question, if you don’t mind. In one of your comments, you mentioned “modern managed languages” in opposition to C++. Now, I’ve been getting the impression that you’re not too fond of Java (which I would regard as at least more recent than C++), either, so could you give some examples what languages you’re thinking of or which ones you’d recommend? Note that I have very little experience with programming, so I don’t really know what the more subtle benefits/detriments of C++ or Java are.

  58. David Arno January 27th, 2011 12:32

    @Socob,

    There is no simple answer to your question. What do you want to do with the language? Which platforms do you want to target? Which languages do you enjoy using? Is speed or maintainability more important to you? Do any patterns particularly appeal to you? Do you like to develop using the command line or do you prefer to stay within an IDE?

  59. Socob January 27th, 2011 22:30

    I was afraid something like that would be the answer. Generally, for my purposes, I’m not unhappy with what I’m using right now, which is Delphi – of course, if you have some kind of opinion on that, I’d be glad to hear about it.

    However, I was intrigued when you talked about said unnamed “modern” languages, because it sounded like you had something specific in mind. Additionally, at least to my knowledge, there haven’t really been any ground-breaking newcomers to the group of programming languages since Java (the only more recent thing worth mentioning that I can think of is C#, which I know nothing about), so I was surprised when you complained about Java’s age/lack of modernity.

    Another thing is that with Delphi, sometimes I feel like things are more complicated to do than, say, in PHP. Now, PHP probably isn’t the best example for a “good” language, but its arrays with string indices (array["a"]) is something that springs to my mind. It’s not the only example, but more like a general feeling I have.

    So, really, I don’t know exactly what I’m looking for. I just have this feeling that there’s something I don’t know about and that I’m missing out on. I’m sorry that you’ve had to put up with my ramblings, but if you can make some sense of it, I’d appreciate it if you could comment on it.

  60. Broady March 6th, 2011 22:51

    Don’t use “private”, don’t use protected either. Document props and methods as private.

    Don’t type props. Let everything be objects, only document variables as what their interfaces are.

    And namespaces – who needs them if You can just put info in docs…

    Sorry for sarcasm, but I just found it as the shortest way to describe my opinion about this idea. What You encourage, David, I think we call a philosophy of liberal programming, where user is not hampered in any way, he’s free and can do what he wants with the libraries. I agree with that.

    Sometimes, however, a developer doesn’t want to let users use his library beyond his own functionality. Sometimes users want the same – mainly because when there are no private properties, then everything is private – I don’t know which parts I shouldn’t change on my own, so for future compatibility I don’t change anything. Similarly, if I’ll read in an instruction of an electric kettle that I can do whatever I want with it, then I’ll be scared to touch it with wet hands, as opposed to an instruction that clearly state what’s dangerous.

  61. Broady March 6th, 2011 22:52

    only document classes* as what their interfaces are.

  62. David Arno March 7th, 2011 13:19

    @Broady,

    You offer a reductio ad absurdum fallacy as your argument and thus it is unfortunately a non-argument regardless of whether you are being sarcastic or not.

    If a developer doesn’t want a user to use his library beyond his own functionality, then the correct thing to do is to mark the classes as final or sealed. If a class is not marked that way, the developer is implicitly allowing the user to extend and override the built-in functionality. In this case, avoiding private and using protected instead encourages the developer to document how the class works from the perspective of a user wishing to extend and override the built-in functionality.

    In the nine months since I wrote this article I have been examining both my own and other people’s code with regard to the use of the private keyword. What has struck me the most is that it is harder as a library developer to use protected as one must document better. Private is the lazy developer’s tool of choice…