Finishing the Iteration

Keeping in mind your newfound skills in refactoring, tackle the remaining stories with an eye toward removing duplication and making the code simpler.

Story 14: Changing the Week Recalls a Previous Week

Recalling state is not hard because the ContactVisit state practically begs it. We quickly get halfway there by adding the following function to customer-contacts.php:

   function gatherContactVisits ($dbh, $emp_id) {        $result = $dbh->query ("select * from contact_visits where    emp_id = ".$emp_id." and week_start = '".getCurrentStartWeek()."' order    by seq");        if (DB::isError($result)) {            return array();        }        $visits = array();        while ($row =& $result->fetchRow()) {            array_push ($visits, new ContactVisit($row));        }        return $visits;    } 

You should be lazy about calling the gatherContactVisits() function because the act of recalling ContactVisits() from the database might not be required each time; for instance, when ContactVisits() are being saved.

Now assign the contact visit data by reference to ContactVisits and recall it for display like so in the customer-contacts.tpl:

   <input name="company_name_{$smarty.section.idx.index}" size="20" maxlength="50"    value="{$contactVisits[idx]->company_name}"> 

You've got everything except for the reload if the week is changed in the drop-down. Here is a cute trick; drop-downs don't normally cause submits to happen but in this case you need to force it.

In order to force a submit, you create a hidden form with the week_start value defined in it. When the date is changed, call the JavaScript function reload, which in turn populates the week_start with whatever was selected. Then, "submit" it with a new action.

   {literal}    <SCRIPT TYPE="text/javascript">    <!--    function reload () {       window.document.forms[0].week_start.value =    window.document.forms[1].week_start.value // hidden form       window.document.forms[0].submit(); // hidden form    }    // -->    </SCRIPT>    {/literal}    <h3>Customer Contact Report</h3>    <form action="customer-contacts.php" method="post">    <input type="hidden" name="action" value="reload_contact">    <input type="hidden" name="week_start" value="">    </form>    <form action="customer-contacts.php" method="post">    <table border="0" width="100%">    <tr><td><b>Employee Name:</b></td><td>{$user->first_name}    {$user->last_name}</td>    <td><b>Department:</b></td><td>{$user->department}</td></tr>    <tr><td><b>Number:</b></td><td>{$user->id}</td>    <td><b>Start Week:</b></td>    <td><SELECT NAME="week_start" onchange="reload()">{html_options    values=$start_weeks    output=$start_weeks selected=$current_start_week}</SELECT></td></tr>    </table>    <br><br><hr> 

Story 15: Per-Week Items on the Customer Contact Report

Remember, the elements of concern are Number of Shop Calls, Number of Engineer Calls, Number of Distributor Calls, Approximate Mileage, Territory Worked, and Territory Comments. All these occur on a per-week basis.

Realize that you're looking at a many-to-one ratio of contact visits to the new structure: Contact.

   CREATE TABLE contact (      emp_id integer NOT NULL default '0',      week_start date NOT NULL,      shop_calls integer default NULL,      distributor_calls integer default NULL,      engineer_calls integer default NULL,      mileage decimal(9,2) default NULL,      territory_worked varchar(60) default NULL,      territory_comments text    );    CREATE UNIQUE INDEX co_pk on contact (emp_id,week_start);    CREATE INDEX co_emp_id ON contact (emp_id);    CREATE INDEX co_week_start ON contact (week_start); 

Not a problem, because Contact is used in much the same way as ContactVisits. Create a test for contact persistence:

       function testContactPersistence() {            $this->_session->getDatabaseHandle()->query("delete FROM contact WHERE    emp_id = 1 and week_start = '1980-01-01'"); // remove multiples            $c = new Contact (                array ("emp_id"             => "1",                       "week_start"         => "1980-01-01",                       "shop_calls"         => 2,                       "distributor_calls"  => 3,                       "engineer_calls"     => 4,                       "mileage"            => 50,                       "territory_worked"   => "Central Ohio",                       "territory_comments" => "Buckeyes are great. "),                $this->_session->getDatabaseHandle());            $result = $this->_session->getDatabaseHandle()->query("select * FROM    contact WHERE emp_id = 1 and week_start = '1980-01-01'");            $this->assertEquals(0, $result->numRows());            $c->persist();            $result = $this->_session->getDatabaseHandle()->query("select * FROM    contact WHERE emp_id = 1 and week_start = '1980-01-01'");            $this->assertEquals(1, $result->numRows());        } 

Contact also shares the same scope as ContactVisits. This is a big hint because the two classes do pretty much the same thing and share the same scope; they're typically an easy refactoring target. Looking deeper into the classes, you see that your hunch is correct and that the new Contact class, and the existing ContactVisit and WidgetUser classes, all contain common functionality.

As you may recall from earlier, the footprint of ContactVisit and WidgetUser is

   class ContactVisit {        protected $contentBase = array();        protected $dbh = null; // database handle        function __get ($key) {}        function __construct ($results, $dbh = null) {}        private function isEmpty($key) {}        public function isValid() {}        private function implodeQuoted (}        private function generateSqlInsert ($tableName, &$metas, &$values) {}        public function persist() {}    }    class WidgetUser {        protected $contentBase = array();        protected $dispatchFunctions = array ("role" => "getrole");        function __construct($initdict) {}        function __get ($key) {}        function __set ($key, $value) {}        public function getRole() {}        public function isSalesPerson()  {}        public function isSalesManager() {}        public function isAccountant()   {}    } 

This is a great use of the "Extract Class" (Refactoring, p. 149) refactoring in order to consolidate the code. The consolidation moves a lot of persistence functionality up into a new class called PersistableObject.

   class PersistableObject {        protected $contentBase = array();        protected $dbh = null; // database handle        protected $dispatchFunctions = array ("role" => "getrole");        function __get ($key) {            // dispatch by function first            if (array_key_exists ($key, $this->dispatchFunctions)) {                $funcname = $this->dispatchFunctions[$key];                return $this->$funcname();            }             // then state             if (array_key_exists ($key, $this->contentBase)) {                 return $this->contentBase[$key];            }            // then self            return $this->$key;        }        function __construct ($results, $dbh = null) {            $this->dbh = $dbh;            if ($results <> null) {                $this->contentBase = $results; // copy            }        }        public function implodeQuoted (&$values, $delimiter) {            $sql = "";           $flagIsFirst = true;           foreach ($values as $value) {                if ($flagIsFirst) {                    $flagIsFirst = false;                } else {                    $sql .= $delimiter;                }                if (gettype ($value) == "string") {                    $sql .= "'".$value."'";                } else {                    $sql .= $value;                }           }           return $sql;        }        public function generateSqlInsert ($tableName, &$metas, &$values) {            return " insert into ".$tableName.                "        ( ".implode              ($metas, ", ")." ) ".                " values ( ".$this->implodeQuoted ($values, ", ")." ) ";        }        public function generateSqlUpdate ($tableName, &$metas, &$values) {            $sql = " update ".$tableName." set ";            for ($i=0; $i<count($metas); $i++) {                $sql .= $metas[$i]." = ".$vaules[$i].", ";            }            return $sql;        }        public function generateSqlDelete ($tableName) {            return " delete from \"".$tableName."\" where ".$this->getSqlWhere();        }        // note: should be implemented by concrete classes        public function getSqlWhere() {            return "";        }        public function isValid() {            return true;        }        public function persistWork ($tablename, $meta) {            if ($this->isValid() == false) return false;            $values = array();            foreach ($meta as $mvalue) {                array_push ($values, $this->$mvalue);            }            if (strlen($this->getSqlWhere()) > 0) {                $sql = $this->generateSqlDelete ($tablename);                $this->dbh->query($sql);            }            $sql = $this->generateSqlInsert ($tablename, $meta, $values);            if (DB::isError ($this->dbh->query($sql))) return false;            return true;        }    } 

You may have noticed that the addition of generateSqlUpdate and generateSqlDelete allows PersistableObject to respectively update and delete itself in the database. Of course, both of these functions require a SQL WHERE clause so that not every record in the table gets updated and deleted. This is why child classes (such as ContactVisit) have the responsibility of defining what makes them unique; they do this by implementing the getSqlWhere function.

By moving the persistence functions (implodeQuoted, generateSqlInsert, isValid, persistWork) and persistence state (contentBase, dispatchFunctions) up into PersistableObject, then Contact, ContactVisit and WidgetUser can all be implemented in a much shorter form:

   class Contact extends PersistableObject {        function __construct ($results, $dbh = null) {            parent::__construct ($results, $dbh);        }        public function persist() {            return $this->persistWork ("contact",                                array ( "emp_id",                                        "week_start",                                        "shop_calls",                                        "distributor_calls",                                        "engineer_calls",                                        "mileage",                                        "territory_worked",                                        "territory_comments"));        }        public function getSqlWhere() {             return " emp_id = ".$this->emp_id." and week_start = '".$this-    >week_start."'";        }    }    class ContactVisit extends PersistableObject {        function __construct ($results, $dbh = null) {            parent::__construct ($results, $dbh);            static $sequence = 0;            $sequence = $sequence + 1; // increment across class            $this->contentBase["seq"] = $sequence;        }        private function isEmpty($key) {            if (array_key_exists($key, $this->contentBase) == false) return true;            if ($this->contentBase[$key] == null) return true;            if ($this->contentBase[$key] == "") return true;            return false;        }        public function isValid() {            if ($this->isEmpty("emp_id") == true) return false;            if ($this->isEmpty("week_start") == true) return false;            if ($this->isEmpty("company_name") == true) return false;            return true;        }        public function persist() {            return $this->persistWork ("contact_visits",                                array ( "emp_id",                                        "week_start",                                        "seq",                                        "company_name",                                        "contact_name",                                        "city",                                        "state",                                        "accomplishments",                                        "followup",                                        "literature_request" ));        }    }    class WidgetUser extends PersistableObject {        function __construct($initdict) {            parent::__construct ($initdict);            $this->dispatchFunctions = array ("role" => "getrole");            $this->contentBase = $initdict; // copy        }        public function getRole() {            switch ($this->contentBase["role"]) {                case "s": return ("Sales Person");                case "m": return ("Sales Manager");                case "a": return ("Accountant");                default: return ("");            }        }        public function isSalesPerson() {            if ($this->contentBase["role"] == "s") return true;            return false;        }        public function isSalesManager() {            if ($this->contentBase["role"] == "m") return true;            return false;        }        public function isAccountant() {            if ($this->contentBase["role"] == "a") return true;            return false;        }    } 

Boy, it sure is great to have all those unit tests to indicate when something goes wrong. One obstacle you'll face is deciding how your target classes are used and how they differ from one another. For instance, Contact and ContactVisit have subtle differences in that you persist only valid ContactVisits, but there is always a possibly empty but still instantiated Contact.

Also notice that Contact implements the getSqlWhere function so that it can be deleted and updated; however, remember that ContactVisit gets deleted before insertions take place and WidgetUser is only persisted to the database, thus not requiring it to implement the getSqlWhere function.

However, the differences between Contact and ContactVisits have less to do with the classes themselves than with the business rules and the environment they exist in. Therefore, even though your classes are starting to consolidate and share code, their respective unit tests should continue to test individual class diversity.

Okay, customer-contacts.tpl now needs a new section in order to display the new Contact information:

   <table border="0">    <tr>     <td>Number of Shop Calls:</td><td><input name="shop_calls" size="7"    maxlength="17" value="{$contact->shop_calls}"></td><td width="20"></td>    <td>Number of Engineer Calls:</td><td><input name="engineer_calls" size="7"    maxlength="17" value="{$contact->engineer_calls}"></td>    </tr>    <tr>    <td>Number of Distributor Calls:</td><td><input name="distributor_calls"    size="7" maxlength="17" value="{$contact->distributor_calls}"></td>    <td width="20"></td>    <td>Approximate Mileage:</td><td><input name="mileage" size="7" maxlength="17"    value="{$contact->mileage}"></td>    </tr>    <tr>    <td>Territory Worked:</td><td colspan="2"><input name="territory_worked"    value="{$contact->territory_worked}"></td>    </tr>    <tr>    <td colspan="7">Territory Comments:<br><TEXTAREA NAME="territory_comments"    ROWS=4 COLS=95>{$contact->territory_comments}</TEXTAREA></td>    </tr>    </table> 

Add these new support functions to customer-contacts.php in order to read and save the Contact information:

   function persistContact (&$dbh, $emp_id) {        $c = new Contact (            array ("emp_id"             => $emp_id,                   "week_start"         => getCurrentStartWeek(),                   "shop_calls"         => $_REQUEST["shop_calls"],                   "distributor_calls"  => $_REQUEST["distributor_calls"],                   "engineer_calls"     => $_REQUEST["engineer_calls"],                   "mileage"            => $_REQUEST["mileage"],                   "territory_worked"   => $_REQUEST["territory_worked"],                   "territory_comments" => $_REQUEST["territory_comments"]),            $dbh);        $c->persist();    }    function gatherContact (&$dbh, $emp_id) {        $result = $dbh->query ("select * from contact where emp_id = ".$emp_id."    and week_start = '".getCurrentStartWeek()."'");        if (DB::isError($result)) return array();        return new Contact ($result->fetchRow());    }    $user = $session->getUserObject();    // display    if ($_REQUEST["action"] != "persist_contact") {        $smarty->assign_by_ref ("user", $user);        $smarty->assign_by_ref ("contact", gatherContact($session->    getDatabaseHandle(), $user->id));        $smarty->assign_by_ref ("contactVisits", gatherContactVisits($session-    >getDatabaseHandle(), $user->id));        $smarty->assign('start_weeks', getStartWeeks());        $smarty->assign('current_start_week', getCurrentStartWeek());        $smarty->assign("max_weekly_contacts", $GLOBALS["max-weekly-contacts"]);        $smarty->display('customer-contacts.tpl');        exit;    }    // persist contact visits    require_once ("lib/contact.phpm");    persistContact ($session->getDatabaseHandle(), $user->id);    persistContactVisits ($session->getDatabaseHandle(), $user->id); 

See Figure 22-7 to view results of your hard work.


Figure 22-7

The speed at which the last story was accomplished indicates that if you keep your code base clean by not going into code debt, your forward momentum will not necessarily fall by the wayside.

Refactoring is something that you should strive for and in a way is never complete. There are sections that could still have a critical eye applied to them, notably those with striking similarities in the support functions for Contact and ContactVisits. The reason the support functions exist at all, as opposed to having the objects take care of everything themselves, is an indication of how their shared scope will probably lead to more consolidation.

Regardless, that is still something that could go in a different direction, and you have to remember that in order for a particular refactoring to be successful, the resulting code should be easier to read and understand. If you find yourself spending more time writing excessive support plumbing or a framework, take that as an indication that maybe you should stop for a moment. Although large refactorings are sometimes needed, they should definitely be the exception.



Professional PHP5 (Programmer to Programmer Series)
Professional PHP5 (Programmer to Programmer Series)
ISBN: N/A
EAN: N/A
Year: 2003
Pages: 182
BUY ON AMAZON

flylib.com © 2008-2017.
If you may any questions please contact us: flylib@qtcs.net