RE: [PEAR-DEV] Re: [PEPr] Comment on PHP::Mailman

2) I don't think that's what Till was talking about... constructors in general return the object. As far as PhpDocumentor is concerned, you don't need to have any @return at the constructor. 3) DOM has loadHTMLFile(), which could be used for some ancient markups. You could try to see if it works here. If it fails... yeah, I can see why you'd be forced into regexes. The only alternative would be to use Tidy before parsing, but that's not enabled by default. I have something additional to say in regards to Till's 1st point... this is just a personal opinion (in general... not in regards to your package), but I find it a good practice to use protected for properties UNLESS you have both setters and getters for that property (protected or public). If you do have such getters and setters, it makes more sense to keep the property private. And it makes sense to have getters and setters if the property's value can't be arbitrary, but instead validate to some constraints. For example, a property that is a URL needs to be a valid URL, so it makes sense for it to be private, and have a public or protected getter and setter. Whether to sanitize at output or validate at input (or both; or the opposite) is a whole other issue that I personally deal on a case-by-case basis. Note however that "neither" is not exactly a good option (as per the scenario outlined in 6). Regards, Vasil Rangelov -----Original Message----- From: HM 2K [mailto:nfhm2k@gmail.com] Sent: Thursday, September 08, 2011 2:03 PM To: Till Klampaeckel Cc: PEAR developer mailinglist Subject: [PEAR-DEV] 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 >