Programming Psychology II: Private methods

Posted by Felix Geisendörfer, on Jul 07, 2008 - in Coding Techniques & Tools » Techniques & Habits

Hey folks,

my previous posts about code that is easy to write vs code that is easy to read and why XHTML is a joke spawned a fair amount of criticism. This time I am afraid very few people are going to agree with me at all:

Private / protected methods and properties are one of the most stupid concepts of OOP.

This is a thought I first shared at CakeFest Orlando this year, but could not explain properly at the time.

Here is the typical excuse for why any language should implement such a terrible concept:

php
  1. class BankAccount{
  2.   private $balance = 0.00;
  3.  
  4.   public function set($newBalance) {
  5.     if (!is_numeric($newBalance)) {
  6.       $this->error = 'And I will strike down upon thee with great vengeance
  7.               and furious anger those who would attempt to poison and
  8.               destroy my bank account. [...]';
  9.       return false;
  10.     }
  11.     $this->balance = $newBalance;
  12.     return true;
  13.   }
  14.  
  15.   public function get() {
  16.     return $this->balance;
  17.   }
  18.  
  19.   public function save() {
  20.     // Great, I can now always blindly trust the value of $this->balance!
  21.     return file_put_contents('balance', $this->balance);
  22.   }
  23. }

Awesome! Now nobody can mess with the balance of this account class and you don't even have to communicate the rules for using it to other programmers!

If you think you can manage programmers or enforce API policies with a few keywords of your programming language ... think again!

If somebody does not understand why he is not supposed to modify the balance property - he will find ways around it. And it won't be pretty, trust me. He'll directly write his values to the database and reload the object. He'll simply change your source code without warning. He'll extend the class and overwrite the set method.

Programmers will do just about anything to restore power you are trying to take away from them.

Now you might think: "But it really makes sense to use a private property here, nobody would ever want to work around this?". Well maybe. And thats a big maybe. Predicting what other programmers will want to do with your code is like playing number guessing with uuids.

So why not try to make the other programmers your allies? Show them what you are trying to accomplish in a semantic fashion:

php
  1. class BankAccount{
  2.   public $balance = 0.00;
  3.  
  4.   public function validates() {
  5.     if (!is_numeric($this->balance)) {
  6.       $this->error = 'And I will strike down upon thee with great vengeance
  7.               and furious anger those who would attempt to poison and
  8.               destroy my bank account. [...]';
  9.       return false;
  10.     }
  11.     return true;
  12.   }
  13.  
  14.   public function save() {
  15.     // Lets just make sure that nothing has gone wrong before saving
  16.     if (!$this->validates()) {
  17.       return false;
  18.     }
  19.     return file_put_contents('balance', $this->balance);
  20.   }
  21. }

This approach has many advantages:

  • It gets rid of the useless overhead from setter / getter functions.
  • It communicates that you need to validate certain things before saving a record.
  • It embraces open architectures and others can work with it right away.
  • It makes your class more flexible since you can temporarily feed it with invalid values.
  • It exposes a new useful method.

Is that it?

Despite the fact that I think that private / protected are a stupid idea to begin with, I have an even bigger issue with them:

The concept of private / protected properties and methods seems to be the most popular recipe for producing crappy code.

I think it's safe to say that all of us were following at least one of the patterns listed below at one point:

  • Not sure where this code goes - I'll put it in a private method for now.
  • Oh, I will refactor this later, so I temporarely put it in a private method.
  • This is just a helper function, no need to clutter the API with it.
  • I am sure no other class will need to access this property ... lets make it private.

Now ask yourself how many times those decisions have lead to code you are proud of? I cannot recall a single occasion in my work as a programmer where using private / protected has helped me to write better code. In fact, these days I even judge other peoples code by it:

When I see more than 2-3 private / protected methods in a class I know the code I'm looking at is in poor shape.

This may sound like a big simplification, but it holds true. I've almost never seen people use private / protected in the proper (yet stupid) way they are supposed to be used.

The psychology behind this is simple. Give people a way to ignore things they don't want to deal with and they will. Private / protected were not meant for that purpose, but unfortunately they encourage the worst habits in us programmers and I therefor highly recommend against using them.

-- Felix Geisendörfer aka the_undefined

PS: Even if you "just" use the CakePHP convention of prefixing functions with one or two underscores to indicate scope visibility you'll end up writing messy code.