| Re: A suggested refactoring in core Xoops classes and XoopsObject |
| by lubdub on 2004/1/12 18:22:28 Thx for your kind answer. Hoping to hear more on this subject soon ;) |
| Re: A suggested refactoring in core Xoops classes and XoopsObject |
| by skalpa on 2004/1/11 14:51:08 Yes m8, Thanks, and my apologies for the lack of answer (but we're quite busy at the mo and aren't answering to all posts even if we're reading them  ).Actually, I've already done something like this some time ago except that the additional data (id fieldname, table name...) was passed using additional constructor parameters and the queries autogenerated looping through the XoopsObject vars. Extremely simplified, it looks like this: le="color: #000000"><?php function &get( $id ) { $tgt =& $this->create(); $vars = implode( ",", array_keys( $tgt->vars ) ); $query = "SELECT $vars FROM {$this->table} WHERE {$this->idfield}=$id"; ...... } But this hasn't been sent to the actual kernel as Kazu is experimenting a solution that would be more optimised (a bit like yours, except that all the data access code would be generated by the XOOPS kernel during module installation / object types registration, so modules writers don't even have to write the methods by themselves). Also, there are some problems in the actual XoopsObject interface (i.e: the default format for getVar() should be 'n', etc...) and we're actually wondering if this wouldn't be best to offer a new / enhanced class instead of building on the actual XO layer. But thanks again. I believe we'll be really back soon, and ideas like this are always welcome. So stay around, you may have others ones we hadn't already .Skalpa.> |
| Re: A suggested refactoring in core Xoops classes and XoopsObject |
| by lubdub on 2004/1/11 13:29:55 Up... Did you Core devs had a look on it? |
| A suggested refactoring in core Xoops classes and XoopsObject |
| by lubdub on 2003/11/1 15:55:48 When porting a module of my site from XOOPS to Xoops², I crawled through the classes and it occured to me that some db/object accessing code is replicated throughout many classes. I wrote my own WhoswhoItem class (my module is a Who's who module for my student association), which extends XoopsObject and is the base class for all my other objects. I think the following code is something that could (should? ) be included in the next RC/release inside the core XoopsObject class itself.The base principle is that it uses the java-reflection-like capability of php and assumes proper naming conventions to create the concrete class instances. If this becomes a performance bottleneck, the createInstance method can be made abstract or overriden. (forgive my typos, my own coding conventions, and so on) le="color: #000000"><?php /** * @abstract */ class WhoswhoItem extends XoopsObject { function WhoswhoItem() { $this->XoopsObject(); } } /** * @abstract */ class WhoswhoItemHandler extends XoopsObjectHandler { var $concreteClassName; function WhoswhoItemHandler(&$db) { $this->XoopsObjectHandler($db); // Of course, it assumes a correct implementation // enforcing the naming conventions $handlerClassName = get_class($this); $this->concreteClassName = substr($handlerClassName, 0, -7); // 7 is the length of 'Handler' } /** * Returns the table used for persistance. * @abstract */ function getTableName() { } /** * Returns the complete Sql request for an Insert operation op the concrete handled subclass. * @abstract * @param int $id the id generated for this instance. * @param object &$toStore the instance to store. */ function getInsertRequest($id, $toStore) { } /** * Returns the complete Sql request for an Update operation op the concrete handled subclass. * @param object &$toStore the instance to store. * @abstract */ function getUpdateRequest($toStore) { } /* * Returns the column name of the primary key used to * retrieve instances from the table. Usuall 'id', or 'uid', .... * @abstract */ function getPKName() { } /** * Creates an instance of the concrete handled class. * @access protected */ function &createInstance() { $className = $this->concreteClassName; $value = new $className(); return $value; } /** * Creates a new instance of the concrete handled subclass. * @return bool $isNew Flag the object as "new"? * @access public */ function &create($isNew = true) { $value =& $this->createInstance(); if ($isNew) { $value->setNew(); } return $value; } /* * Retrieves data from DB and fills a new instance the concrete handled subclass with it. * Assumes the PK id is an int. * @access private * @param int $id ID * @return object instance, FALSE on fail */ function &get($id) { $sql = sprintf( "SELECT * FROM %s WHERE %s = %d ", $this->db->prefix($this->getTableName()), $this->getPKName(), intval($id)); if ( !$result = $this->db->query($sql) ) { return false; } $numrows = $this->db->getRowsNum($result); if ( $numrows == 1 ) { $value =& $this->createInstance(); $value->assignVars($this->db->fetchArray($result)); return $value; } return false; } /** * Stores a the concrete handled subclass. * @param object &$toStore an instance the concrete handled subclass. * @return bool TRUE on success */ function insert(&$toStore) { if ( get_class($toStore) != $this->concreteClassName ) { return false; } if ( !$toStore->isDirty() ) { return true; } if (!$toStore->cleanVars()) { return false; } if ($toStore->isNew()) { $generated_id = $this->db->genId( $this->getTableName()); $sql = $this->getInsertRequest($generated_id, $toStore); } else { $sql = $this->getUpdateRequest($toStore); } if (!$result = $this->db->query($sql)) { return false; } if (empty($generated_id)) { $generated_id = $this->db->getInsertId(); } $toStore->assignVar( $this->getPKName(), $generated_id); return true; } /** * Deletes an instance the concrete handled subclass. * @param object &$toDelete * @return bool TRUE on success */ function delete(&$toDelete) { if (get_class($toDelete) != $this->concreteClassName) { return false; } $sql = sprintf("DELETE FROM %s WHERE %s = '%s'", $this->db->prefix($this->getTableName()), $this->getPKName(), $toDelete->getVar($this->getPKName())); if (!$result = $this->db->query($sql)) { return false; } return true; } /** * Deletes all instances matching a set of conditions * @param object $criteria {@link CriteriaElement} * @return bool FALSE if deletion failed */ function deleteAll($criteria = null) { $sql = 'DELETE FROM '.$this->db->prefix($this->getTableName()); if (isset($criteria) && is_subclass_of($criteria, 'criteriaelement')) { $sql .= ' '.$criteria->renderWhere(); } if (!$result = $this->db->query($sql)) { $this->notifyUpdate($sql, 'failed_update'); return false; } return true; } /** * Retrieves multiple instance of the concrete handled subclass. * @param object $criteria {@link CriteriaElement} * @param bool $id_as_key Use IDs as array keys? * * @return array Array of {@link XoopsGroupPerm}s */ function &getObjects($criteria = null, $id_as_key = false) { $ret = array(); $limit = $start = 0; $sql = 'SELECT * FROM '.$this->db->prefix($this->getTableName()); if (isset($criteria) && is_subclass_of($criteria, 'criteriaelement')) { $sql .= ' '.$criteria->renderWhere(); if ( '' != $criteria->getSort() ) { $sql .= ' ORDER BY '.$criteria->getSort().' '.$criteria->getOrder(); } $limit = $criteria->getLimit(); $start = $criteria->getStart(); } $result = $this->db->query($sql, $limit, $start); if (!$result) { return $ret; } while ($myrow = $this->db->fetchArray($result)) { $value =& $this->createInstance(); $value->assignVars($myrow); if (!$id_as_key) { $ret[] =& $value; } else { $ret[$myrow[$this->getPKName()]] =& $value; } unset($value); } return $ret; } /** * Obtains the "db-quoted" version of the variables contents, to be able to store them directly. Just a helper method, not mandatory. * @access protected */ function "edCleanVars(&$object) { $toReturn = array(); foreach ($object->cleanVars as $k => $v) { $toReturn[$k] = $this->db->quoteString($v); } return $toReturn; } } With this, in subclasses, you don't need the write the typical "get", "getObjects", "delete", ... methods. You only need to write: o a Constructor o the concrete implementation of getTableName() o the concrete implementation of getInsertRequest() o the concrete implementation of getUpdateRequest() o the concrete implementation of getPKName() All of which are quite simple and quite short methods. This makes your classes shorter to write and easier to maintain. |
Search
Recent Posts
Who's Online
Donat-O-Meter
| Stats | |
| Goal: | $15.00 |
| Due Date: | Jul 31 |
| Gross Amount: | $0.00 |
| Net Balance: | $0.00 |
| Left to go: | $15.00 |
|
|