Re: [PECL-DEV] New chmlib PHP binding

On Sat, 14 Aug 2010 01:40:33 +0100, Jesús Espino <[email protected]>  
wrote:

> Hi:
>
> My name is Jesús Espino, and i'm programming my first PHP Extension,
> that is a binding of the chmlib for easy access to the chm files.
>
> The code can see here  
> https://redmine.jesusespino.org/projects/phpchm/repository
>
> I want to include it on the PECL repository, but first i want to get
> suggestion of changes (i have tried to follow the coding style and
> recomendations of PHP).
>

OK, here are a few:

* You barely have any error handling!
* You don't need to use a resource and store it in a property in the  
object. You can store the pointer directly in the object structure.
* Don't use __destruct for object cleanup. Use the store's "destruct" or  
"free object" callbacks. The first one is called only if the object was  
correctly constructed (the constructor didn't throw an exception).
* Neither your class, nor your constructor are final. In PHP,  
unfortunately, subclasses are not required to call the parent constructor.  
This is particularly serious for internal classes. If you don't want to  
make the constructor/class final you can either:
   * Check on each method call if the constructor was called, and if not  
throw an exception. This is the option most commonly used.
   * More cleanly, but with more boilerplate code, replace the  
get_constructor handler, make it return a wrapper function and have this  
wrapper function call the actual constructor (original or the subclass  
override). After calling it, the wrapper function checks if the  
constructor was called and throws an exception otherwise. See here:  
http://wiki.php.net/internals/engine/objects#designing_subclassable_classes

-- 
Gustavo Lopes