Re: Some PEAR remarks

Vincent Oostindie wrote: > > Hi there, Hi Vincent, > A question in advance: why are member variables in PEAR prefixed with a > '_'? Just a way to make more clear to the user: "this property is private, please do not use it". > And if you use proper encapsulation, member > variables are never accessed from anywhere else. (And very, very sadly, > this is not the case in the PEAR library!) Could you tell where in the PEAR library? > Let's start at the beginning: with class PEAR. This class is supposed to > be the base class of all new classes (with non-static members). Examining > this code led me to the following observations: The PEAR.php class is supposed to be the *error handling* class not the base class. > - Class PEAR is large. The file 'PEAR.php' (although containing two > classes) is almost 800 lines. <kidding> $ wc -l PEAR.php 805 $ php -w PEAR.php | wc -l 2 </kidding> > - Error handling is built in. That doesn't lead to a very clear separation > of 'normal' code, and 'error' code. Class PEAR is bloated with error > handling methods (isError, setErrorHandling, expectError, popExpect, > raiseError, pushErrorHandling, popErrorHandling). In my opinion, all error > handling code should be separated from the PEAR class. As I said before, the PEAR class provides exactly that: error handling. >Even more so > because PHP is an interpreted language, and any program simply hasn't got > errors once it's finished, resulting in a lot of 'dead' code. With a clear > separation between 'normal' code and 'error' code, the latter can be > easily removed once the program is completed, which speeds it up > tremendously (less code to parse, less checking to do, faster execution). ... a lot of bugs :-) > - The member variable $_debug defines if the class is in debug-mode. > However, there is no way this value can be set BEFORE an object of class > PEAR is instantiated. This var was put there in the early stages of the class for just debuging purpouses. It should be removed now at it has no effect and nobody should use it (remember our "_var" convention?). Thanks for remember us it. > - Subclasses of class PEAR cannot know if their superclass is in > debug-mode ($_debug == true), After the explanation above that is no longer valid. We agree no? > With that said, I'll move on to the database classes, as these are the > most popular. Have you analyzed more classes? I would like to hear more comments about other classes, please go ahead. > Class DB is a class with static methods only, and the two most important > ones (factory and connect) either return an object of the requested type, > or an instance of class 'PEAR_Error'. Again, I think this is not a good > separation of 'normal' code and 'error' code. Also, whenever some code > calls one of these methods, it should always check whether the object > returned is the one they wanted, or something else. This raises a > question: how often is this done by your typical lazy programmer? Just one time at the top of the code: <?php PEAR::setErrorHandling(PEAR_ERROR_DIE); $db = DB::connect(); $res = $db->query(); while ($res->fetchInto()) { } ?> > And what > about production-state code? That kind of code doesn't contain any > programming errors, so then there's no need to check for them. The other day I was reading an interview to a one popular FreeBSD core developer. He said that the success for his fixes and robust code was putting assertions everywhere, even where is not supposed to be needed. And I can not agree more with him. And if you talk me in the specific case of databases, men, the number of things that can happen are too high for not doing an extensive cheking everywhere. Again, the "bloated" PEAR error handling will do all the job for you. > Class DB is the entry point to creating connections to any kind of > database. How many applications need that much abstraction? Not a lot, I > think. I take that as your personal situation. I use 6 or 7 different databases each day, and beleive me, I do need such abstraction. > Class DB_common, like class PEAR, is very large. Too large, if you ask me. > The problem with desiging classes is always which features to put in it, > and which to leave out. In my opinion, class DB_command has way too many. > How many programmers will use 'sequences'? Very few, I gather (at least I > certainly won't.). So put them in a separate class. If someone needs them > they can easily be included, but if they're not needed, they are not > loaded into memory. This is true, and we have already discussed that (btw is not as easy as you think if you want to preserve the speed and usability). We will change that in the future. > Finally, I'd like to point out some other thing regarding PEAR: lack or > reuse. One of the most important features of object-oriented programming > is that code can easily be reused. With PEAR, this is not the case. I find > that OO is mainly used to define class interfaces, and not to make reuse > simple. Not only should PEAR be used outside of the library (in > applications) over and over again, this should also be the case for code > in the library itself. The reason why this isn't possible right now is > that a lot of methods are very big. By dividing those big methods in > smaller ones, it's much easier to reuse them. Sorry if I don't catch this point. You can't reuse them because they are big? And example could help me here. > Well, that's about all I have to say about PEAR right now. Note again that > all points I made are personal, subjective observations. Most of you will > probably disagree, and probably rightly so. Well I mainly disagree with most of your points. I guess that you haven't really tried to use PEAR and the bounch of things it provides. Hope you have understood my explanations. Regards, Tomas V.V.Cox