The Symfony and Doctrine2 project recently changed protected functions and properties into private in order to provide a well defined public API. While I agree with the principle, this has some bad side effects for FLOW3 code.
In the TYPO3 project we learned the Open / Closed Principle the hard way. Because TYPO3 started as a PHP3 project more than ten years ago, all methods and properties were public by default – and extension developers made heavy use of many functions of the TYPO3 core which were never meant to be public. This is actually one of the reasons we finally decided to rewrite TYPO3 from scratch – because any refactoring of the core was like playing with the fire, as we couldn't know how many extensions were relying on the part we were going to change.
The least we could do was using the public and protected keywords to document which code parts could be used by other classes. However, sub classes still might rely on protected methods which are not meant to be part of the publicly supported API. The logical next step is to mark those methods as private. Problem solved?
When we discussed using the private and final keywords two or three years ago, we concluded that this wouldn't solve our problem completely: there are still public methods (which need to be public because they are used by other classes in our framework) which we don't consider part of the public API. So as we cannot prevent usage of these functions technically, we chose to go for a white list approach: all classes, methods and properties which we consider part of the public API (and hence make sure that we stay backward compatible) are labeled with an @api annotation in the respective doc comment. Our API documentation contains only those code parts and give you a clear overview of what API you may use.
But why not use final and private keywords for the rest? Because it gives us some headaches with our proxy mechanism. As you might know, FLOW3's AOP implementation (and since recently also our DI support) relies on proxy classes and hence the ability to extend and override the original class. This works fine as long as your class is not final and its methods are either public or protected. While there certainly is a way to circumvent this (I know one, but it makes me shiver), life would be so much easier without private and final.
So the question remains: should FLOW3 support and use private / final methods and classes? Or do @api annotations make more sense to you?
Michael | 14.03.2011
@Robert: Love the idea with the AOP advice :) That could solve the problem of breaking someone else's TYPO3 installation ... cool.
@Lukas: With a system like this in place, what's wrong with letting people "shoot themselves in the foot"? Also, if someone "accidently" shoots himself in the foot by simply not caring about @api annotations, I doubt this person would know how to fork sth ...
Lukas | 11.03.2011
Whats wrong with forking + sending an upstream patch? But sure there are some cases where its tricky. Like we do not have read only properties and in some cases one might be forced to make some method public for internal purposes. So sure the @api annotation is a nice compliment, but I don't see how relying on that is enough, since users will accidentally still shoot themselves in the foot.
Christopher Hlubek | 11.03.2011
Private and final is really a blocker to code reuse and extension. Some projects and libraries in the Java world (including core J2SE libraries!) made these mistakes a long time ago. Proteced methods can often serve as template methods for changing behaviour in subclasses and could also be considered API methods.
I like the way FLOW3 tackles this problem (besides the technical necessity) and would favor this approach in other projects too. And there is really (almost) no reason to go for final methods IMHO. So please PHP developers don't go and make the same mistakes Java developers made before. Good code communicates and doesn't need strict language restrictions.
Robert Lemke | 11.03.2011
@Michael I could imagine that we provide a tool / mechanism in FLOW3's development context which logs any use of non-API functions by third-party code (actually that would be possible by creating a little AOP advice).
This would give developers warnings at an early stage and we could also use it for some QA checks before publishing packages in a public repository at typo3.org or the like.
Michael | 11.03.2011
I'm doing a lot of JavaScript development at the moment and because there are no visiblity modifiers there, you also have to come up with a solution how to handle the issue. I'm currently using the underscore convention for all "internal" properties and everything else is public API. That way, you also show that some methods, though used by other parts of the framework or application, are not intended as public API. So basically, that's the same as the @api annotation. +1 for that :)
But, something to think about: @api will tell developers what to use, yes, but will it stop them from using the other stuff? Probably not. Now one could say then it is their own problem, but I'm not so sure about that: If they do this in an extension available in the repository, than that extension might be used on many websites and updating TYPO3 will cause it to break ...
Christian | 11.03.2011
Maintenance (of public API) surely is a concern for any major open-source project so Doctrine has a point there. But closing the code down won't help the project so the way to solve this concern is wrong to me. Having a clear statement (like the @api annotation) what is part of thepublic api keeps maintenance down and leaves the code open to modification if the need arises (in most cases AOP should be enough though).
Claus Witt | 11.03.2011
In my opinion private should be used to refactor code from other methods. for instance if several methods use the same algorithm, or if one methods algorithm will become more readable (and thus easier to maintain). In these cases the code should be seen as part of the calling method(s) - but just branched into their own (private) method to ease reading and maintaining.