Re: Some PEAR remarks

Vincent, Quite interesting. Your thoughts are quite complete, and they do make some good points. I find there are some flaws in your arguments, particularly with your "Too Large" remarks, as well as in regards to what PEAR can do in designing an OO library, seeing that PHP simply isn't as OO as many (including I) would like it to be. My points are below. But like I said, your points are well taken. ----- Original Message ----- From: "Vincent Oostindie" <news@sunlight.tmfweb.nl> To: <pear-general@lists.php.net> Sent: Wednesday, April 17, 2002 7:11 AM Subject: [PEAR] Some PEAR remarks > 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 I have some serious doubts about that, as serveral of your statements leave me to believe that you haven't fully explored the PEAR website, and learned why certain things are done certain ways. > 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!) http://pear.php.net/manual/en/standards.naming.php > > 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. I don't see how you think 380 lines of code is Pretty big? 800 lines of code? Yes, with comments, and those are some pretty big comments. > - 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). This may be the case to some extent, but at the same time, your assertion that once a program is finished, it hasn't gotten any errors in it is not entirely true. Maybe a better method of handling errors is in order. > - 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. (And doing it afterwards makes little sense and > violates encapsulation as well.) Also, when a program is in debug-mode, > it's almost always the whole program that should respond accordingly, and > not just a couple of objects. This is one of the very few occassions where > using a global variable ('_DEBUG_' or something) could be justified. - > Using debug code slows down the program even when $_debug == false, > because of the necessary if-statements. With a simple trick, this isn't > necessary at all: replace the if-statements with a call to a function or > method inside an assert-statement (e.g. 'assert($this->debug('PEAR > constructor called'));' This function should always return true. It has a > couple of advantages: 1) better separation of debugging code and normal > code, 2) if assertions are disabled in php.ini, the function-call is > simply skipped, adding no overhead to production code, 3) subclasses can > easily add debugging code specific for that class, 4) there's no need to > use a global variable as described in the previous remark. The only > disadvantage of this approach is that debugging code gets a bit slower (a > function call takes longer than an if-statement), but that doesn't matter > much, because it's debugging code. > - Subclasses of class PEAR cannot know if their superclass is in > debug-mode ($_debug == true), unless they access that member variable > directly. If you aim for proper encapsulation, subclasses should never be > doing that. Also see the previous remark. - All in all, I think class PEAR > should mainly do one thing: add destructor capability to classes. Having > destructors can be very useful (although often misused). > > 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? This is a moot point, considering if they don't check what they are getting back, they aren't really doing their job now. > 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. I wonder if you have ever worked in actual production then? Databases go down, connections get changed, things happen. If an error occurs somewhere along the line, I need to know about it. The idea that once code is in production it doesn't contain errors, or errors won't occur, is asking for trouble. > > 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. How would this be better? Rather than return an error, which is what happened, we return a DB Object? > > 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. Your then most likely going to want to use the actual PHP functions for speed improvement anyways. > 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. How are you defining too large? > 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... Actually, it's much less than that. Don't mix lines of code with lines in the file, as many of these files contain more comments than code. Also, you assertion to make a simple query is true, it would be too much. But name me a project that simply does 1 query? Sure, for a small, simple project, PEAR might be overkill. But then, what would be the need for PEAR in such a project? > > 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. This is where your PHP inexperience starts to show. Unfortunately, PHP never claimed to be a OO language. Obviously, in ZE2 they are trying to fix that, and a lot of the things you discuss here can be done once it comes out, but right now, technologically, it's not possible. Now granted, personally, I don't like the way PEAR handles the DB interface. In fact, for some odd reason, seeing as I never really dabbled much with ASP, I like the ADODB style from Jon. However, for my work, I use the actual database functions for mere speed. Maybe PEAR DB will be redesigned when ZE2 comes out. > > 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. Again, I find your Bigger/Smaller arugments flawed as mentioned above. > > 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. > > Vincent > > -- > PEAR General Mailing List (http://pear.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php >