Vote Details for "Net_WebFinger" by till

» Details
Voter: Till Klampaeckel [Wishlist] Vote: +1 (not conditional) Reviews: Deep source review
» Comment
Overall really well done. 1) What I'm not so sure about is the request code in the Net_WebFinger class. Some of the methods seem long and a little redundant in terms of code. Maybe spin it off into a dedicated request class. I have no idea yet, if you catch my drift, maybe you can improve this, otherwise I'll take a look later and will send you a pull request. 2) Net_WebFinger::finger() should IMHO throw an exception if no '@' is found. 3) I'd see if you can use Net_URL2 (which comes with HTTP_Request2) for the "isHttps()" check. Speaking of which, is SSL usually a given with these things or the exception? If most of the resources are non-SSL, I'd run the non-SSL request first. Otherwise it'll be pretty expensive. And/Or offer a way to force "non-SSL" lookup (with finger()). 4) I'm not so sure if I like Net_WebFinger_Error. Are all errors usually to be disregarded? Is that why you don't throw Exceptions? Or is there another reason I am missing. 5) I'd like to see an autoloader. I'm not sure why you think PEAR1 cannot have this. Especially with a 5.3 minimum, require_once seems really outdated and would force me to fork for my own uses to use another class-loading strategy. Imho, an autoloader could take care of all that. Enabled by default, and if people want to run their own, they could disable it. Then there's no need to strip require_once. 6) Careful on the dependency on Cache. ;) (I think we need to work on something there and bring the code up to date.)