links

buttons

It seems like the Symfony2 CS is forbidding the use of private methods. I think other frameworks have similar rules. Now I have argued along those lines myself in the past. But especially talking to the Doctrine2 dev's got me to reconsider .. a bit. This all kinda of reminds me about the discussion over making it a fatal error to change method signatures during inheritance. I love PHP for allowing me to do this, yet giving me an E_STRICT, aka red tape, to make sure I am aware of the kittens that are being strung up this very moment. When I have coded myself into a corner, I need to get out of it quick, I do not need

I still do not believe in private, but I do believe that it would be a good idea to keep the user in the loop what the extension points are that you designed and where you commit to maintaining BC and where you don't. In many cases users may want to change a private method simply because they didn't understand the design of the API (often because they are too lazy to read the docs to learn how to do things right). Without some indication which methods are public, protected and private users would not know that they are over stepping red tape and get upset if methods are changed later on. So again I think it would be awesome to just get a slap on the wrist, aka an E_STRICT, when violating visibility rules. Since this is not the case, I actually kind of like the CakePHP approach using single/double underscore's for visibility constraints.

Now I should explain a few more things about the benefits doing the right thing™ and the reasons for still sometimes not doing the right thing™. In general if an API is well designed use of private can make total sense. Basically you are saying this part of the code is not extensible, it hasn't been designed as such. Such a great designed API should however have more sensible extension points to still get the results you want. Similarly, breaking the "is a" relationship by screwing with the signature of methods will eventually bite you in the ass too, because suddenly assumptions about what you can do with an object because it is an "instance of" something will blow up in your face. Doing the right thing is the right thing most of the time.

However the reality of project life is that some times you need to shot from the hip to get things done in time and on budget. Sometimes this might code you into a corner, but it will still be good enough for the project to be a success. Again this is why I appreciate the red tape, that way I cannot accidentally do the wrong thing™, it will have to be a conscious decision and one that I cannot easily hide from the client (if they have any knowledge of PHP). I do not want to hide anything from the client, I want them to be aware of the compromises I make. But if their time and budget does not allow for a perfectly thought out design or when a time saving library simply doesn't allow me to do the things I need to do for a client, then I want to be able to overstep the red tape, because it is the best thing to do for my client. There might be clients that rather want to spend the time, and I very much welcome these kinds of clients. But I fear most of these clients rather go to a Java shop for that kind of work and they are willing to accept the additional required development time and budget requirements.

I also want to present another angle. The internet thrives on the idea of innovation at the edges. People put out things and others pick them up, twist and turn them until they do things never imagined possible by the original people. More importantly things move way faster than the original people ever anticipated and chances are good that they will have a hard time keeping up with their users. Having to wait for some upstream project to integrate some patch, or even just agreeing that a change is necessary is often just not possible in the context of a client project. Of course DVCS like git have made all of this more possible: You simply fork on the spot, easily keep up with upstream changes and when things slow down, you can contribute the changes back. But what happens if they never accept your changes or add the feature in an incompatible way? Then you are stuck with maintaining your fork, which will become am increasing pain if they make BC breaking changes upstream, just like if you might see BC breaks if you rely on extending private methods (assuming the language would allow it). So I much prefer if I could have some nagging E_STRICT remind me of the places where I over stepped the red tape.

Now knowing when it is ok to overstep the red tape, knowing the chances of this biting you in the ass, knowing where to look when things are biting you in the ass etc.: These are all things that set apart the good from the great PHP developers. This is the main difference between a junior and a senior developer and they have to take responsibility when things go wrong. Anyway, I guess the point with use of private is that you really need to know what you are doing to end users and you better spend enough time designing a very good API because you basically have taken away the ability of the end user to fix your oversights.

Comments

I can see your point in where you think private may make sense, but overall I agree with you that private is not a good thing. At least not for open source libraries that were built for people to build upon.

Still, as a project, you can draw lines. I would draw a very easy and clear line: The public API will be maintained as backwards compatible, the protected API can be used for extending the classes but the behaviour of those methods may change at any given point (though effort will be made not to break them). That way, there is no real reason for final or private: You have simply given a clear statement on the why and how of each access modifier.

Now, in customer projects, however, things are different. These projects are (usually) not built purely for building upon, instead they are meant for the end user to use: Here it might make sense to make methods final and private, because there is a very clear view on what it will be used for. And even if you end up in a situation where you need a change, there is no problem for it to change. In an open source library, changing the access modifier or method is a bit harder because people may rely on a certain way of doing things.

Open Source libraries can not cater all valid use cases, so even if you think you can cater to most of the valid use cases and your API is perfect for every situation, there might be another one (and probably is). Whether it's bad practice for that developer to do it, or they have another valid use case that you didn't think of, by making things private you actually block those people from building in their use case. And that's a shame, because that means they either have to fork or wait for the main project to adopt their use case.

The only real downside I can find for not using private methods is that developers are less inclined to share their specific use case and their solution to the problem they encountered. If something is protected and they can simply extend, then it works for them in their code. When a method is private and they have to make changes, developers usually would like these changes to be adopted upstream so they'll make a ticket and submit a patch. This is the only positive point I see where using private methods in Open Source libraries would be a benefit :)

I am in the camp of "design and document your classes for inheritance or else prohibit it".

However, I don't have a problem with the approach of forbidding private and making everything protected, it is just important that it is sold as what it is: a fragile way to allow people to work around issues through class inheritance. So fragile that such implementations are not even guaruanteed to continue to work after bugfix releases. (I am talking of all the classes that are made non-final/all-protected-no-private, not about classes that are explicitly designed to be extended and thus where great care is taken for backwards compatibility).

Advertising this as "extensibility" is just wrong imho. You obviously can not maintain backwards compatibility for all your classes so that you don't break subclasses. Many people don't even know how difficult it is not to break subclasses in some way when changing parent classes. Just because you don't change the code in a protected method does not mean you don't break it in some way. It is so easy to (accidentily or not) change the semantics or the way in which the superclass does some things which can cause very subtle and hard to find bugs in the subclasses.
Inheritance introduces extremely strong coupling between classes, so much that many people rightly speak of inheritance breaking encapsulation.

After thinking through it a bit more, I really think that the issues I see with "private" in the real world can be solved by DVCS's and services like github. Find an issue, fix it, submit a pull request, keep in sync with upstream until the changes are merged, done .. without preventing library authors to steer people towards extension points.