Reducing Duplicate Code

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Post Reply
Cirdan
Forum Contributor
Posts: 144
Joined: Sat Nov 01, 2008 3:20 pm

Reducing Duplicate Code

Post by Cirdan »

In my current project, I have many places where I have duplicate code that looks similar to the following two snippets

Code: Select all

	public function insertPage($title, $body) {
		$table = $this->getPageTable():
			
		$data['title'] = $title;
		$data['body'] = $body;
		$data['last_update'] = time();
		
		$result = $table->insert($data);
			
		return $result;
	}
	
    public function updatePage($title, $body, $id) {
        $table = $this->getPageTable();
        
		$data['title'] = $title;
		$data['body'] = $body;
		$data['last_update'] = time();
		
        $where = $table->getMySQL()->quoteInto('id = ?', $id);
        $result = $table->update($data, $where);
        
        return $result;
    }
Here the code is 99% the same except one updates an existing entry, and one inserts a new entry. How do I reduce this duplicate code? Is it worth the extra code and complexity to write a method that can handle both cases, then wrap that method into these two methods? This particular situation only has these two methods, but in my Forum code, there are several more like this. Minor differences, but a majority of the code is the same. Maybe this duplication means something is wrong with my design?
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: Reducing Duplicate Code

Post by Eran »

Those seem like class methods. In that case I would usually have an internal class method that prepares the data, something like:

Code: Select all

	public function insertPage($title, $body) {
                $table = $this->getPageTable():
                $data = $this -> _prepare($title,$body);       
                $result = $table->insert($data);
                       
                return $result;
        }
       
	public function updatePage($title, $body, $id) {
		$table = $this->getPageTable();
                $data = $this -> _prepare($title,$body);       
		$where = $table->getMySQL()->quoteInto('id = ?', $id);
		$result = $table->update($data, $where);

		return $result;
	}

	protected function _prepare($title,$body) {
		return array(
			'title' => $title,
			'body' => $body,
			'last_update' => time()
		);
	}
User avatar
allspiritseve
DevNet Resident
Posts: 1174
Joined: Thu Mar 06, 2008 8:23 am
Location: Ann Arbor, MI (USA)

Re: Reducing Duplicate Code

Post by allspiritseve »

Cirdan wrote:Here the code is 99% the same except one updates an existing entry, and one inserts a new entry. How do I reduce this duplicate code? Is it worth the extra code and complexity to write a method that can handle both cases, then wrap that method into these two methods? This particular situation only has these two methods, but in my Forum code, there are several more like this. Minor differences, but a majority of the code is the same. Maybe this duplication means something is wrong with my design?
This looks very similar to a Table Data Gateway.

The first thing you could do to remove duplication is instead of passing each variable as a separate parameter, pass it in an array, like so:

Code: Select all

public function insert($page) {
   // insert
}

public function update($id,$page) {
   // update
}
That frees you up to do fancy things like looping through the array to build a SET string for your INSERT and UPDATE queries.

Once you do that, however, you'll find that you have similar code between your page gateway, your post gateway, your article gateway, etc. The next step then is to take the code that builds queries from arrays, and put that into a parent gateway that all of your concrete gateways extend from. You might want to place generic queries in there like findById, insert(), update(), and delete() as well, since all you really need to generate those queries is a table name.

To start with, many of your concrete gateways will look like this:

Code: Select all

class PageGateway extends Generic_Gateway {

  public function __construct($db) {
    parent::__construct($db,'pages');
  }

}
That should be a good start.
Post Reply