[PEPr] Comment on Web Services::Services_Facebook

Travis Swicegood (http://pear.php.net/user/tswicegood) has commented on the proposal for Web Services::Services_Facebook. Comment: Cool... now I don't have to write one :-) Just running commentary as I go through it: * What about moving the connection data out of Services_Facebook_Common into Services_Facebook_Connection, the making the Common class instantiate the connection? That allows for changes to happen specific to the connection without touching every single object (by inheritance). I'm specifically thinking of Services_Facebook_Connection_Driver classes to handle curl, sockets, pecl/http, etc., etc. * I would move Services_Facebook_Common::checkRequest() into a Services_Facebook_Util class as a public so it can be explicitly tested * The Users::$userFields array needs to be accessible somehow. I'm thinking of the cases where you want everything but one or two fields, or in future cases where you want everything and a few new fields. Making it public doesn't seem like a great idea though. Possibly making it protected so you could sub-class S_F_Users and add your own default fields. How about this: make $userFields protected, and add a public static $defaultUserFields that is used on each instance to start the $userFields. * Services_Facebook_Photos::$imageTypes is similar to above. It's not as big of deal here, but just for future extensibility when the hot new image type is supported and you've moved on to Python or Ruby :-) * What is Services_Facebook::isValidRequest() doing? I don't see a call to it anywhere in the code. Has grep failed me? The only other thing is more stylistic and has more to do with a personal preference. With that preface out of the way, I would want to instantiate Services_Facebook and use an API along these lines: <code> $facebook = new Services_Facebook('my-api-key', 'my-secret'); $facebook->groups->getMembers('gid'); $facebook->photos->getAlbumsByUser('some-uid'); $facebook->users->getLoggedInUser(); $facebook->users->isAppAdded(); </code> That reads more like what is seen in the API docs. The various properties could be lazy-loaded via __get(), removing the need to explicitly call factory() all together. To write an API that I would like to use versus the standard Facebook API, I would add classes such as Services_Facebook_User (no 's') that would be my interface to the rest of the world. <code> $user = $facebook->getCurrentUser(); // complemented by a $facebook->getUser('uid'); $user->isFriendsWith('facebook-user-id'); $user->getInfo(); $user->groups; // Services_Facebook_Group[] $user->groups['gid|group name']->members['uid|handle'] instanceof Services_Facebook_User $user->publishStory('title', 'body'); $user->publishAction('title', 'body'); $user->albums['aid|name']->photos; // Services_Facebook_Photo[] </code> That might be something better suited to building on top of a Services_Facebook package as Services_Facebook_Extended. :-) Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=513 -- Sent by PEPr, the automatic proposal system at http://pear.php.net