Re: [PEPr] +1 for Web Services::Services_GeoNames

On Sat, 2008-12-13 at 12:46 +0100, David Jean Louis wrote: > Hello Mike, > > > Michael Gauthier (http://pear.php.net/user/gauthierm) has voted +1 on the proposal for Web > > Services::Services_GeoNames. > > > > Proposal information: > > http://pear.php.net/pepr/pepr-proposal-show.php?id=550 > > Vote information: > > > > http://pear.php.net/pepr/pepr-vote-show.php?id=550&handle=gauthierm > > > > Comment: > > > > Looks good to me. > > > > I'm not certain it's a good idea to make your HTTP_Request2 object a > > public property. In my opinion, it would be better served as a protected > > property with a public setter method. > > why ? getters/setters are just evil imho ;) > The main benefit with using a setter in this instance would be separation of implementation from interface. Public properties are both interface and implementation. If something needs re-factoring in the future, you may have to break a public API (or forgo re-factoring). > > > > More specific exception types would be useful, as would documenting under > > which conditions exceptions are thrown in the @throws sections. > > Not sure what other exceptions the package should raise, what do you had > in mind ? > I guess the only other exception type I see needed here is Services_GeoNames_HttpException. That way, you could distinguish between GeoNames API errors and HTTP errors. Regards, Mike