Vote Details for "Services_UseKetchup" by doconnor

» Details
Voter: Daniel O'Connor [Wishlist] Vote: +1 (conditional) Reviews: Cursory source review
» Comment
Bunch of trivial nitpicks; sorry for not commenting earlier! I know you've done it to save you writing code, but work in the constructor scares me a touch :S - $this->getApiToken(); Ditto instantiating classes in places that aren't the constructor - makeRequest() for instance. Also having some of the unit tests incomplete because a certain user account doesn't work :( PHPUnit's getMock() or HTTP_Request2's Mock Adapter ftw. From an overall API perspective; it might be worth splitting up the "create me a bunch of classes, wire them together with these credentials" behaviour into a different class (Services_UseKetchup_Factory) from the "execute these API calls and give me data" code in the core class. I can see you've sort of done it with Services_UseKetchup_Common; but I'd encourage you to really define the purpose of each class, and polish its responsibilities. Look forward to seeing it in PEAR.