Re: [PEPr] Comment on PHP::Mailman

Hi Till, I do appreciate your input, but there were a couple of issues: 2) No documentation on __construct was found on phpdoc.org: http://www.google.co.uk/search?&q=site:phpdoc.org%20__construct 3) I have considered it, but I find regex more practical, especially for the ancient markup mailman uses. Perhaps I'll provide a case study at some point to better illustrate my point. For now I'll run with "for the sake of performance". You can view the current code here: https://github.com/hm2k/PHP_Mailman/blob/master/PHP/Mailman.php I'm glad you like the concept, that is reassuring. Thanks to you both so far for the input, I feel like it has turned into a mature package already. On Wed, Sep 7, 2011 at 8:15 PM, Till Klampaeckel <till@php.net> wrote: > Hey, > > a few suggestions: > > 1) Don't use private, use protected. > > 2) Update your docblocks (phpdoc.org has a lot of documentation -- e.g. a > __construct doesn't return void, it returns the actual class.) > > 3) Consider ext/dom to parse html (instead of preg_match) > > 4) Get rid off user_error(): if the error can be neglected. I'd collect > them in the class, add hasError(), getError() style methods. If those > errors should be addressed, use an exception. > > 5) Instead of file_get_contents(), use HTTP_Request2 so people can use > different transports and customize it, etc.. E.g. right now it's not just > hiding possible errors but I'm also unable to use a different extension, > proxy or similar. > > 6) I'd make adminurl and adminpw protected and add set-methods for various > reasons: it looks like both require a little validation (e.g., is this a > URL, does this password not contain spaces, etc.). Setting the public var > could lead to unexpected things. > > > All in all, I like the idea very much. I had the need for something like > that a couple times before. > > Cheers, > Till > > -- > http://pear.php.net/pepr/pepr-proposal-show.php?id=665 >