Re: Re: Getting rid of "underscore for private methodnames" codingstandard

Christian Weiske wrote: > Hello Greg, > > >>> 2. Having an underscore preceding private methods and >>> class variables makes it impossible to open up the API without >>> breaking existing code. >> This assertion is untrue because by definition private >> methods/variables are only used internally and are not a part of the >> API, thus they can be changed/renamed at will using a search/replace >> with no consequences. If one wants to expose a previously private >> variable or method, as far as external users are concerned, this is >> no different than adding a new variable or method. > > I think I made myself not clear enough: > By "breaking code" I do not mean to break a public API. I mean making a > method public as easy as possible. The easiest way is to replace > "private" with "protected" or "public", and it's done. > Currently, with a _ in front, you'd have to remove that and *change > your code* (in this class) because it's broken now. That's what I meant > with "breaking existing code". I see. Having actually performed this change multiple times when moving PEAR code to Pyrus, I can say with confidence that this is trivial to do. Any decent programming environment allows a search/replace where you can preview the changes (including grep/sed), and with code that conforms to PEAR CS already, it's highly unlikely that one will encounter a naming conflict in the search/replace. This, again, is from my experience actually doing it, and not a hypothetical. I'm sure one could find a hypothetical where this would be hard to do. However, I have found benefit in my own coding with using _names. Also, I used to *hate* this standard when we moved phpDocumentor into PEAR, but it actually helps debugging, because seeing a ->_blah tells you right away what kind of method/variable this is. >> However, I think a *far* better suggestion would be to forbid the use >> of the private keyword unless the intention is to prevent any possible >> modification because of race conditions or other critical coordination >> issues. Instead, we require all variable to be protected or public >> unless there is extreme justification. In my experience working with >> other people's code, the "private" keyword simply makes it impossible >> to implement code reuse without resorting to vast swaths of cut/paste. > > That would mean that everything is public API, thus "breaking API" will > happen in most releases when internals change, and keeping BC is nearly > impossible *except* the class layout was finalized before releasing the > first beta. This is really hard, since internal changes like method > reorganization or splitting of internal methods are almost impossible to > do then without breaking the API, because everything is public API. > > How do you justify that? Or do we need a new definition of BC? Again, this is from my experience, but I have never seen a private variable or method that was actually private when one is extending the class in question. I *have* seen variables/methods that would be useless to extend in the most common use cases, but that is different. The most common problem I've encountered with both old PEAR-style PPP and language-enforced PPP is that "private" is abused. For truly useful packages, there is always a use case that the original author didn't envision. As an example: there is a rewrite of the PHPT runner that is intended to supplant the existing one in the php-src source tree. This is a refactoring from the ground up into object-oriented code, and was intended originally to solely run the .phpt tests in the php source tree. However, we've been using phpt inside PEAR packages for years, with our own runner, also based on the original PHPT runner, but with some extensions for PEAR-specific stuff. Helgi wrote some code that allows generation of code coverage using xdebug, which requires modifying the way --FILE-- sections are handled. When I set about trying to extend the new PHPT runner to do this, it seemed easy. The --FILE-- section was handled with a class, so all I needed to do was extend that class and add in the xdebug stuff, right? To my dismay, I quickly discovered that it had been designed so rigidly (every variable was private, and most methods too), there was no way to do this without extending *10* unrelated classes, cutting and pasting more than 200 lines of code into the child classes just so I could add or change 1 line of code for each method (one cannot call private methods on a parent class), and in the end gave up and requested that they move all private to protected declarations. This then made it possible to do what I wanted in about 35 lines of code. The difference is astounding, considering the only source code change was replacing every "private" with "protected." As to the question of BC, there is a trade-off to everything. I prefer greater flexibility at the risk of making BC harder to preserve, because you gain nothing by making an inflexible class that is easily modified drastically in future releases. You gain a great deal from a class that may be harder to drastically change without breaking BC, but probably doesn't need to be modified at all because extending it to add new features is trivial - isn't this after all the point of using OO? All this verbiage is not because I ultimately care that much about private variable names, I just think the question is kind of irrelevant since private variables and methods should not be used in all but the rarest of situations, since protected variables and methods are far more flexible and still provide code/variable isolation from accidental external meddling. >> This will have the side effect of doing the exact naming suggestion >> you've made, since the current CS forbids _naming with protected >> variables/methods. > Thank you for the interesting idea, that's something what I hoped to > gain with the mail to pear-dev@ :) My pleasure, it's arcane stuff like imagining future repercussions that I enjoy most about programming :). Greg