how to avoid duplicated code?

Question

I've the following Problem, those methods of my SQLBuilder are simply the same, what can I do to reduce the code?

    public function select($fields){
        if(is_array($fields)){
            $this->fields = implode(',', $fields);
        } else {
            $this->fields = $fields;
        } 
        return $this;
    }

    public function from($tables){
        if(is_array($tables)){
            $this->tables = implode(',', $tables);
        } else {
            $this->tables = $tables;
        } 
        return $this;
    }

http://stackoverflow.com/questions/108699/good-php-orm-library — Denis de Bernardy, Jun 26 '13 at 11:59
change all your code from `->from` to `->select` ? As Jeremy noted its called refactoring. Search and replace all will become your best friend unless you happen to use a decent IDE with good refactoring built in (hint php doesn't have one) — Dave, Jun 26 '13 at 12:04
If the methods are the same changing all your froms to selects will mean you can then delete the from method. you're making better reuse of an existing function and removing non-required code — Dave, Jun 26 '13 at 12:08
this is an SQL builder later the objects are used to create a query like echo $query->select('field1')->from('table1')->where('data<10')->getQuery(); — Nextar, Jun 26 '13 at 12:08
which since your tables and fields methods are identical you can basically do this $query->select('field1')->select('table1')->where('data<10'); and get the same result but this is where refactoring comes in you have to modify the parent class to allow reuse of the same method multiple times in your query construction process. its fairly simple. either that or you leave it exactly as is and you live with duplicated code. — Dave, Jun 26 '13 at 12:11
@Dave Calling `select` twice won't give the same result, the methods aren't the same. One of them sets `$this->fields` and the other one sets `$this->tables`. — nmclean, Jun 26 '13 at 12:25
Shouldnt matter as thats never called outside of that just being the returned var waygoods answer below is basically the same thing but converting teh select and from to wrappers for a generic create method. his method saves updating all the code in favour of just updating the class outcome is the same though. — Dave, Jun 26 '13 at 12:27
@Dave It's not the return value, it's an instance variable. It does matter because the instance is directly modified as a result of calling the methods. `$query->select('field1')->select('table1')` leaves `$query` in a different state than `$query->select('field1')->from('table1')` does. — nmclean, Jun 26 '13 at 15:05

score 1 · Accepted Answer · answered Jun 26 '13 at 12:19

create a common private method that does the processing and then public methods that use them

public function select($fields){
    return $this->builddata($fields, 'fields');
}

public function from($tables){
    return $this->builddata($tables, 'tables');
}

private function builddata($data, $storage) {
    if(is_array($data)){
        $data = implode(',', $data);
    }
    $this->$storage = $data; // variable variable

    return $this;
}

score 0 · Answer 2 · answered Jun 26 '13 at 12:42

0

or

public function action($data, $type){
        if(is_array($data)){
            $this->$type = implode(',', $data);
        } else {
            $this->$type = $data;
        } 
        return $this;
    }

then

$this->action($data, 'select');

answered Jun 26 '13 at 12:42

GoldenTabby

220
1
13

how to avoid duplicated code?

2 Answers2