Re: Some PEAR remarks

On Wed, 2002-04-17 at 13:11, Vincent Oostindie wrote: > Hi there, > > I've been browsing the PEAR code to see if I would like to use the library > in my own PHP programming experiences, but unfortunately I must say that - > after examing the code thoroughly - I have decided I will not do that. The > reasons for that decision I have written down here. > > Please note that I am in no way trying to attack your collective > programming and design skills, even though it may seem like that at some > points. These are just my personal observations, and those tend to be > subjective... Also note that at some points I may sound a bit harsh. I > would like to blame this on the fact that I'm not an English native > speaker, so for me it can be hard sometimes to express myself properly in > that language. > > A question in advance: why are member variables in PEAR prefixed with a > '_'? Sure this is useful in languages like Java or C++ where it's hard to > see which variable is a local one, an argument or a member, but in PHP > this distinction is already clear: all member variables are prefixed with > '$this->' inside a method. 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!) Public properties are useful sometimes, I'm not in the "encapsulate or die!" camp, even though I always almost encapsulate. The _debug stuff is just a leftover from the early days like Tomas says. > 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: > - Class PEAR is large. The file 'PEAR.php' (although containing two > classes) is almost 800 lines. I think that's pretty big for a base class. > In my opinion, a base class should be fairly small. If you take a look at > other (large) object-oriented libraries, you'll see that base classes are > always pretty small, and for a reason: every other class in the system > relies on it, so making it big increases overhead as well as the > possibility of bugs. The PEAR class is really not a base class like Object in Java, but a place where we can emulate functionality that is still missing from PHP (such as destructors and exceptions). > - 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. 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). PEAR is very much geared towards ZE2 and PHP 5. PEAR errors are a poor man's exceptions, and they will be replaced completely by exceptions when using PEAR with PHP 5, so using them will become easier. That being said, PEAR errors, as exceptions, should not be abused. They are for reporting failures that are an exception from normal behaviour. For a lot of functions, returning false is perfectly fine, but for errors that either need abstracted handling (errors from DB are an example of that), or are not recoverable, PEAR errors should be used. > With that said, I'll move on to the database classes, as these are the > most popular. > > 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? 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. Uhm, programming errors? PEAR can't help you with those I'm afraid ;-) > Much of the error handling can be simplified by introducting 'Null' > objects. These are objects of a class that simply do nothing. For example, > you could have a class DB_null that is just like any DB_common-derived > class, and is returned whenever a requested database class doesn't exists > (when calling DB::connect). It simply returns default values from its > methods instead of doing anything useful. This trick makes using the > library a lot easier, and of course it applies to much more than just the > DB class. Using existing values for errors is very difficult in a large scale. What if you actually want to return NULL from a function? There are no simple, scalar values available for this, that's why we use objects. What can be done, however, is to create an error type in PHP that evaluates to false. Then both "if (!blah)" and "if (is_error(blah))" will work, but I haven't considered all the side effects yet. > 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. That's not to say a class like class DB isn't a good idea, but I > think a programmer should have an choice: either instantiate a class for > any DBMS through class DB, or instantiate a class for a specific DBMS > directly. True, the latter is still possible, but even when a class like > DB_mysql is instantiated directly, class DB must still be included to get > it to work, so there's little point in doing that. If class DB weren't so > big (like class PEAR) and error handling was separated from the class, it > would be a lot easier to get the behavior I describe here. > > 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. Other examples are the methods limitQuery, getOne and > getRow. That's not to say these methods don't come in handy, but I don't > think they should be in a class that emphasizes on defining an interface > for creating database connections (much less using them). Why not put them > in a separate utility class or something? The classes will become a lot > smaller, simpler, and easier to understand. > > If I - as a user of the PEAR library - wish to use PEAR to create a > connection with a MySQL database, the following files will be included: > 'PEAR.php', 'DB.php', 'DB/common.php' and > 'DB/mysql.php'. That's 793 + 874 > + 1281 + 847 = 3795 lines of code (in PHP 4.2RC4). For fairly trivial > tasks like making a database connection and executing SQL queries, I think > that's a bit much... I agree, there's too much code in there right now. When user-space overloading and aggregation is finally declared stable in PHP, DB will be rewritten to use it. I plan to partition out (and auto-load) big chunks of code that will make the basic connection class much lighter. > It is very clear that the DB classes are derived from the Perl DB classes. > I find that a shame. Perl was never meant to be used as a complete > programming environment, even though it has grown to become one. (On I > sidenote, I seriously question the mental health of anybody who uses Perl > for more than simple scripting...) I think it's better to look at > programming languages with a good OO design, like Smalltalk or Java, to > see how object-oriented libraries should be written. Learning OO from Perl > is, in my opinion, a bit like learning to ride a bicycle from someone who > has been in a wheelchair his whole life. The API of PEAR DB is inspired by Perl DB, but other than that there's no connection. > 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. As with encapsulation, I don't think people should be religious about re-use. Sometimes, avoiding too many dependencies is a valid tradeoff for code reuse. Re-use is good, but not something to get religious over. If you can avoid installing/using a whole new package with 5-6 lines of code, those are well worth it from a maintenance standpoint. - Stig