Friday, April 12, 2013

Why You Shouldn't Use Privates Anymore

For nearly half of my career I thought that hiding member variables and methods with private and protected was one of the best ways to write better code. You have probably been taught in school or have read several well written articles of why using private makes your code better.
In theory hiding the details of the class using a private can have the following benefits:

  • Reduces the complexity of the code by limiting communication between modules.
  • Hides implementation details so that you can focus on the what instead of the how.
  • Prevents access to dangerous variables that could unintentionally cause the class to break.
  • Reduces the scope of a member to just your class.

All though I have found that the above are true there are an equal amount of negative consequences for using private scoped members. The negative impact of this design choice, in my opinion are far worse than the actual positives involved. There are other ways to achieve the above goals without the usage of private members.

Limiting communication between modules is a concept that comes from a psychologist named George Miller.  Millers Law basically states that human beings on average can not handle more than 5 plus or minus 2 ideas, entities or objects at one time. I am always aware of this basic principle when designing both user interfaces and when coding.

It is not necessary to lock the inner workings of the class up so that no one can see whats going on inside. If you opened the hood of a car it wouldn't help for them to hide the spark plugs so you couldn't see how it works. Limiting communication between modules and hiding implementation details can be achieved by using either a purely abstract class or an interface.

Using interfaces to communicate between modules it turns out solves a-lot of the problems that private variables where solving. So the question is why do both? Well, it turns out that most developers have not been using interfaces for this purpose. It was not until Test Driven Development came along that private members have become the enemy.

The main reason that TDD hates private members is due to its negative impact on the test-ability of a class. TDD coders like to take a class and write test code that changes variables, and dependencies in order to run automatic scenarios to verify its functionality. If members and variables are private it only serves to make unit testing harder. Due to the fact that test-ability was not a key metric in traditional software engineering methodology, this was overlooked.

Preventing access to dangerous variables may save some headaches in some situations. However, I think that we should let the developers decide what they should or should not be accessed. If the developer is not accessing an interface and has access to the implementation than changing the wrong variable is not the responsibility of the original class creator.

In practice I have seen private variables cause entire classes to be re-written when all that was needed was an override of a single member variable or a trivial function. There is no doubt that the intent in those cases was to prevent developers from tinkering with things they shouldn't be. Why are we arbitrarily deciding what should or should not be "override-able" in our base classes?

If you really want to reduce the scope of a member variable than interfaces are the way to go. I don't care about the scope of a class member if it is not an interface. So let your interfaces handle communication and stop trying to force good developers from solve problems within your solutions.


2 comments:

  1. Very good assessment. I've just re-created an entire TFT Screen driver class because some low level register settings were "private" which made it impossible to write some optimized functionality for it in a sub class. Overrides, shadows, virtual, pure abstract virtual.... nothing was available as a work around. I had to just copy/paste the entire thing and just out of spite I did a search/replace on "Private", and changed to "Public" (after deleting a double "public" block or two).

    ReplyDelete