Re: New chmlib PHP binding

Thanks. I think i follow all your suggestions, I have pushed the changes to the repository, any other suggestion?? What is the next step to put it in the PECL repository?? Greetings On Sat, Aug 14, 2010 at 3:48 AM, Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote: > On Sat, 14 Aug 2010 01:40:33 +0100, Jesús Espino <jespinog@gmail.com> 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 > > -- > PECL development discussion Mailing List (http://pecl.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > >