The age old private vs protected debate has been re-ignited in the PHP community recently following the decision of Doctrine2 and Symfony2 to make all class methods private until there is a very clear and proven reason to change them to protected or public. The intention is a good one – to ensure they are providing a clear and stable API through intentional and known extension points that they can better test and support. Fabien (the creator of Symfony) points to this benefit in an article of his own explaining the thinking behind the decision. The primary started point of Fabien’s article and the driving thought behind this whole change and philosophy is (emphasis his):
Having a few well defined extension points force the developer to extend your library the right way instead of hacking your code.
The problem is that this kind of thinking is a slippery slope that kills the spirit of programming. It alienates the more pragmatic developers within communities. Telling other developers that you are going to force them to work with your code in some pre-determined “right way” is an incredibly arrogant statement to make. Instead of letting the codebase grow organically and be modified or extended at will, you handcuff developers and send them a different message. Allow me to re-phrase it using my own words:
I know the right way to use this code and I have thought of all the possible use cases. If you don’t agree, then you have to prove me wrong and wait until I either agree or figure out a better right way for you.
This is the real message developers hear when internal code is private scope or marked with the final keyword – a message that they have arbitrarily limited power and ability to make changes where they need to. But it doesn’t stop there.
When you take the position that all methods should be private unless proven otherwise, you plant a seed of fear in the developers that use your code. Fear that whenever they may have to veer off the beaten path to meet a project requirement they won’t be able to. Fear that they might miss deadlines waiting for the “right way” to enable the functionality they need. Fear that they might have to modify or hack the actual file or class they need to change the behavior of to achieve the functionality they want within the project deadline because they are not able to extend it.
Using private scope removes the fun from programming and kills the hacking spirit by creating fear through the knowledge that you will not have the ability to make changes anywhere in the codebase you need to when you need to. You are no longer in control of your own code. You were forced to submit to a nanny state mentality by sacrificing your freedoms to modify the code for the promise of future stability and safety. That’s not a fun place to be when you’re on the receiving end, and it’s not a policy I would ever consider enforcing with my own code. I sincerely hope this line of thinking does not proliferate within the PHP community and infect other projects.
Or turn it around, when extending a class because everything is protected by default I will live in fear that when I need to update that my entire project will blow up in my face.
Seriously swapping out a “private” for a “protected” in a git fork will break how often when you sync with upstream? Very seldom, unless of course something major was redesigned at which point the duck tape “practical” approach would also have failed long ago already.
You are clinging to a time when forking was hard and duck tape coding seemed easier. These days forking is easy, so if you really are sure that upstream is too slow or is ignoring your request, then you can still be “practical” and move on.
There is no arrogance is saying I do not know all possible reasonable use cases yet and more importantly I am humble enough to realize that I might need to make radical changes to my code to make it better than it is today.
The real problem is that you’re using implementation inheritance. By and large, that style of OO has failed!
I don’t think that private or final kill the fun of programming … but, bad use of private or final surely kill it.
When developing using private of final, developer should think deeply what they really want to protect, why to do so and will it have impact for subclasses ?
@Lukas -
I see your side of it, but I don’t think maintaining your own fork and keeping it up to date and in sync is a good solution to the problem. When was the last time you had that many issues upgrading from one stable release to another?
If there are that many radical or breaking changes to a codebase, than it’s a beta (certainly undeserving of a 1.0 release label) and those things are expected to happen. If you’re coding to a beta version, then you will probably have to freeze the version if you can’t spend the additional time fixing things when they break, at which point the debate here doesn’t even matter.
@Lukas — while forking may be easier today, it still means maintaining that fork over time, and additional setup in your application infrastructure to ensure your fork is used over the canonical code. It also means that besides extending the class to modify a workflow, you’re also maintaining a fork of the original code. Basically, this approach means putting more of an onus on the _consumer_ of the code.
Frankly, I’m fine with that — but you have to be up front about the reasons for doing so, and also provide detailed documentation on (a) the supported extension points, and (b) how to go beyond the supported extension points should a developer need to.
Marking a method as private does not kill extensibility. Architecturing your tool with pluggable mindset does not prohibit the usage of private.
Marking it as private means user should never be able to touch, because it may lead to unpredictable situations.
Back to Doctrine 1 we suffered a lot of misused methods creating monsters of unknown behavior. For example: Someone other day reported about a Model with issues. He was using a Versionable of an Internationalizable of a NestedSet. All of that became doable because we allowed entities to me modified by extensions, a single method visibility change.
To sum it up, it turned into a monster, buggy and users abuse of it with their weird requests.
So, keeping a consistent API, allowing right extensibility inclusions with correct planning and measuring its impacts is perfect.
A tool may be extensible, but may not be a college party where everything is possible. Most people think Doctrine 1 is not reliable because it is buggy. Have they ever thought that it’s mainly because of their weird usages?
We don’t kill extensibility by no way. There’re enough patterns that fulfill most of your needs. If you need a different one, it may be:
1- Something that can be changed (as we said, case per case)
2- You haven’t planned enough a solution
To give you another example, people are complaining a lot about a static method that creates its own instances. People wanted to be dynamic, allowing you to extend this class and still take advantage of that static method.
Something that people have not paid attention is that instead of using inheritance, they could easily use composition.
I worked on that change that was revert and generated a lot of discussion. Here is the commit link, so you can look at it: https://github.com/doctrine/doctrine2/commit/04ab0cd8fcb90cf60b9ec764ff5eea7c4cbfbaeb#lib/Doctrine/ORM/EntityManager.php
As you may see, it’s not an arrogant behavior, but rather a lack of architecture planning of the guy that requested that change.
Cheers,
@Guilherme Blanco -
First off thanks for posting and sharing your opinion. Like I said, wanting a better API is absolutely a good and noble goal to have.
My response and general feeling though is that users will still find a way to extend a library to do what they want the way they want no matter what you do. Trying to prevent users from hurting themselves isn’t in the DNA of PHP. It repels users from your project – even good ones that do know what they are doing. Taking the position you have puts your views a lot closer to the Java and .NET circles than PHP ones.
In my opinion, the chance to extend behavior is one of the benefits of OOP. Sure, if you change protected methods, it will break customers code. But who said, that protected methods are stable API?
For me only public methods are taken as stable. And migrating dependencies to new versions is always a situation where you have to check “how and where this breaks my code”. In most cases this should not but might likely happen.
In fact, making “non-API” methods private cancels the free will of a developer to use your code in a way he wants/has to to achieve his goals.
Providing a good subset of tests for the public methods besides comprehensive documentation should be enough to secure intended behavior. You cannot think about all use cases.
With reference to the argument “forking might be easy nowadays” – not everyone is using Git or is used to or has the time or wants to maintain a fork.
I was going to try symfony because I heard good things about dependency injection. I’ve lost all interest.
Why? I’ve been working on an Android application and there’s two things my time on this project has thought me it’s a) I loath private visibility and b) I loath anything final.
I actually find it quite insulting, to me it’s like saying “were smarter that you are” not necessarily true. And if theron demographic of symfony users are stupid or lemmings then education is the issue. But then again I am making quite a lot of assumptions about what symfony wants to achieve. The one thing that I do know is that symfony is not for me, i know only too well that it would drive me up the wall.
@Vance Lucas -
My point of view is based on hard learning the worst way possible: experiencing the problem. We gently allow most of extendable methods inside Doctrine 2, but again, we relax a member visibility when we see a clear need and possibility for that.
Java and .NET are not bad languages and we have a lot to learn with them. The problems we are facing here now are the same ones Java experienced back in 1998 and C# in 2005. We are late, but all that was already surpassed by them. It is important a possible good idea to follow what they learned by experience too. It may not suit well at the end for PHP, but generally, it suits.
@Daniel Kreuer -
There are always others and maybe even better ways to extend things with pure OO. Composition is most of the times a better solution than Inheritance, but people still prefer the “easy way”.
I strongly disagree that protected is a base visibility for a tool. Just as one example: Inside Doctrine 2 UnitOfWork, we calculate the commit order (inserts on DB) through a known graph dependency calculator algorithm called Topological Sorting. Why on Earth would someone want to extend it? Any slightly touch to this algorithm would lead to unpredictable results and most of the times a wrong dependency would be generated. So why expose it for users? I agree that user may want to extend UnitOfWork (which I consider a probability of 0.00001%) and schedule Entities for dirty checking somehow. This method is by no means protected, because there may be a need for it. But again, it’s a case by case analysis.
Cheers,
@Matthew: Well one could say that extending on undesigned extension points is worse than maintaining a fork, because you dont even know that you are potentially shooting yourself in the foot. And the surprises that ensue are a much greater onus put on the consumer of the code.
But I totally agree on being up front about this. As for documenting the extension points. I do not think this needs any special documentation, I mean ideally every sensible use case should be covered. Thanks to the use of private/final the need for documentation is actually reduced.
As for documentation about how to fork etc. I think people who do not know how to do this probably are better off filing a ticket. Then again even “experts” can probably benefit from a somewhat best practice workflow.
It’s more a matter of interface than of fear or protection.
The access level of a method is a contract to a user, not an iron-clad protection. It lets the developer consuming your classes know that your messing with these methods was not a design-time consideration, and could lead to undefined behavior or modify the function of the class in a way you are unlikely to predict.
It’s probably a little paranoid to make it the default, but private should have a stronger presence than it does in a lot of code. I use it to mean “I think this is tied too closely to my implementation to ever be of general utility.”
It can’t possibly kill the spirit of programming, because nothing stops you from modifying that class. It’s a limitation of the contract, not of the actual code.