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!)
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.
- 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).
- 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? 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.
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.
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...
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.
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.
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
|
