Re: Object oriented design
| From: | Jochem Maas | Date: | Wed, 29 Mar 2006 22:44:23 +0000 |
| Subject: | Re: Object oriented design | ||
| References: | 1 | Groups: | php.general |
| Request: | Send a blank email to php-general+get-232875@lists.php.net to get a copy of this message | ||
please consider that my comments could start some kind
of OO holywar, you have been warned ;-)
Richard U wrote:
from a design perspective... which class over here should have the sql to search, update, add and remove groups? source... http://www.codedemons.net/pb/?show=4920none of them - they are all abstract! once you get into writing SQL (or writing code that generates it) your in implementation terratory - which means the code belongs in a subclass of GenericUser (or GenericGroup, depending on whether your modelling a group or user) with regard to the Manager object [concept] (I notice there wasn't one for Users) - I don't like them, It feels like an unnatural split of responsibility, I feel the delete() and add() methods should be non-static member funcs of the GenericGroup class (in the case of 'groups'), and that the rest of the find/search/get/getAll type of functions should be static members of that class. coming back to the delete() method; you can only delete an 'entity' exists ... so doing something like: GroupManager::delete( $groupId ); or $gm = new GroupManager(); $gm->delete( $groupId ); is a bit wrong [IMHO] because the group with id $groupId may not even exist to begin with, rather I would advocate something like (I'm assuming you understand Exceptions in order to given the concept the necessary context - that is to say I don't feel you can make this work cleanly with proper use of exceptions): try { Group::get( $groupId )->delete(); } catch (UnknownObject $e) { // group doesn't even exist (someone trying to hack our script?) } catch (FailedDeletion $f) { // database didn't allow our deletion (some kind of constraint in place?) } catch (DBError $f) { // general database problem } actually It seems that there is a good candidate for create a base abstract class that both GenericUser and GenericGroup ... something like: abstract class DataObject { abstract static public function get($keys); abstract static public function getAll(); abstract static public function find($searchCriteria); abstract public function add(); abstract public function update(); abstract public function delete(); } --------- oh and if I were you I kep an eye on the ZendFramework, if only to see how they are tackling these generic-data-object problems.