6
\$\begingroup\$

Can I get some pointers, critiques and/or comments on the following model? (p.s. performance seems great... tested all methods up to 10,000 records)

<?php if (!defined('BASEPATH')) exit('No direct script access allowed');

class o7th_Model extends CI_Model {

    private $msg;
    private $last_id;
    private $full_qry_count;

    public function __construct(){
        parent::__construct();
        $this->load->database();
        $this->load->library('form_validation');
        $this->load->library('pagination');
    }

    // Setup a list assoc array to return
    /*
     * USAGE - the only array parameter required is the 'table_name'
        self::qlist(array('select'=>'FIELDS', --- should be a comma seperate string of field names
                     'where'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'or_where'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need 
                     'where_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'or_where_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'where_not_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'or_where_not_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'like'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'or_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'not_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'or_not_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'group_by'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'distinct'=>'TRUE/FALSE/NULL', --- TRUE enables SELECT DISTINCT, FALSE/NULL doesnt bother adding it
                     'having'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'or_having'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'order_by'=>array(array('field'=>'FIELD_NAME', 'direction'=>'ASC/DESC'),), --- can have as many of these field/direction arrays as you need, direction is optional
                     'limit'=>array(array('limit'=>'NUMBER_TO_LIMIT_RESULTS', 'offset'=>'NUMBER_TO_LIMIT_RESULTS')),
                     'join'=>array(array('table'=>'TABLE_NAME_TO_JOIN', 'on'=>'EX: a.field1 = b.field2', 'direction'=>'left/right/outer/inner/left outer/right outer'),), --- can have as many of these table/on/direction arrays as you need
                     'pagination'=>array('per_page'=>'', 'page_num'=>''), --- do not use this with limit... you will get undesirable results
                     'table_name'=>'TABLE_NAME')); --- REQUIRED!!!
     */
    public function qlist(){
        $tArgs = func_get_args();
        $tbl = self::prepArgs(1, $tArgs);
        $qry = $this->db->get($tbl);
        if($qry){
            $rs = $qry->result_array();
            $this->full_qry_count = count($rs);
            if(array_key_exists('pagination', $tArgs[0])){
                $rs = array_slice($rs, $tArgs[0]['pagination']['page_num'], $tArgs[0]['pagination']['per_page']);
            }
        }else{
            $rs = null; 
        }
        return $rs;
    }

    // setup a return assoc array details record, only returns 1 record
    /*
     * USAGE - the only array parameter required is the 'table_name'
        self::qdetails(array('select'=>'FIELDS', --- should be a comma seperate string of field names
                     'where'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'or_where'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need 
                     'where_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'or_where_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'where_not_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'or_where_not_in'=>array(array('field'=>'FIELD_NAME', 'value'=>array('VAL1', 'VAL2', etc...)),), --- can have as many of these field/value arrays as you need, value needs to be an array
                     'like'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'or_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'not_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'or_not_like'array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE', 'wildcard'=>'before/after/both/none'),), --- can have as many of these field/value arrays as you need, wildcard is optional
                     'group_by'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'distinct'=>'TRUE/FALSE/NULL', --- TRUE enables SELECT DISTINCT, FALSE/NULL doesnt bother adding it
                     'having'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'or_having'=>array(array('field'=>'FIELD_NAME', 'value'=>'FIELD_VALUE'),), --- can have as many of these field/value arrays as you need
                     'order_by'=>array(array('field'=>'FIELD_NAME', 'direction'=>'ASC/DESC'),), --- can have as many of these field/direction arrays as you need, direction is optional
                     'limit'=>array(array('limit'=>'NUMBER_TO_LIMIT_RESULTS', 'offset'=>'NUMBER_TO_LIMIT_RESULTS')),
                     'join'=>array(array('table'=>'TABLE_NAME_TO_JOIN', 'on'=>'EX: a.field1 = b.field2', 'direction'=>'left/right/outer/inner/left outer/right outer'),), --- can have as many of these table/on/direction arrays as you need
                     'table_name'=>'TABLE_NAME')); --- REQUIRED!!!
     */
    public function qdetails(){
        $tbl = self::prepArgs(1, func_get_args());
        $qry = $this->db->get($tbl);
        return ($qry) ? $qry->row_array() : null;
    }


    public function qinsert(){
        return self::prepArgs(2, func_get_args());
    }


    public function qedit(){
        return self::prepArgs(3, func_get_args());
    }

    // setup db delete, return BOOLEAN

    public function qdelete(){
        $tbl = self::prepArgs(4, func_get_args());
        $this->db->delete('Storage_Users');
        if($this->db->affected_rows() > 0){
            return TRUE;
        }else{
            $this->msg = 'There was an issue removing that record.<br />' . $this->db->_error_message();
            return FALSE;
        }
    }

    // get the last inserted id - only valid on inserts
    public function last_insert_id(){
        return $this->last_id;  
    }

    // return a message if any of the returns above are invalid/false/errored out
    public function message(){
        return $this->msg;  
    }

    // return a number of records based on the query run/ only valid on the selects
    public function fullrecordcount(){
        return $this->full_qry_count;
    }

    // return the pagination links
    public function paginator($base_url, $per_page, $total_rows, $num_links, $uri_segment, $aclass = null){
        $config['base_url'] = $base_url;
        $config['per_page'] = $per_page;
        $config['total_rows'] = $total_rows;
        $config['num_links'] = $num_links;
        $config['first_link'] = '<span class="fa fa-angle-double-left page_num"></span>';
        $config['last_link'] = '<span class="fa fa-angle-double-right page_num"></span>';
        $config['cur_tag_open'] = '<span class="page_num bold">';
        $config['cur_tag_close'] = '</span>';
        $config['next_link'] = '<span class="fa fa-angle-right page_num"></span>';
        $config['prev_link'] = '<span class="fa fa-angle-left page_num"></span>';
        $config['uri_segment'] = $uri_segment;
        $config['num_tag_open'] = '<span class="page_num">';
        $config['num_tag_close'] = '</span>';
        if($aclass != null){
            $config['anchor_class'] = 'class="' . $aclass . '" ';   
        }
        $this->pagination->initialize($config);
        return $this->pagination->create_links();
    }

    // setup all possible arguments for all of the above, return name of the table from the arguments
    private function prepArgs($which, $args){
        if($args){
            try{
                switch($which){
                    case 1: // select
                        return self::setupSelect($args);
                        break;
                    case 2: // insert
                        return self::setupInsert($args);
                        break;
                    case 3: // update
                        return self::setupEdit($args);
                        break;
                    case 4: // delete
                        return self::setupDelete($args);
                        break;
                }
            }catch(Exception $ex){
                $this->$msg .= $ex->getMessage();
            }
        }else{
            $this->$msg .= 'You have not passed in any arguments to your query, please have a look over your code.';
        }
    }

    // setup our edit
    private function setupEdit($args){
        // where clause(s)
        if(array_key_exists('where', $args[0])){
            $w = $args[0]['where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where clause(s)
        if(array_key_exists('or_where', $args[0])){
            $w = $args[0]['or_where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_in clause(s)
        if(array_key_exists('where_in', $args[0])){
            $w = $args[0]['where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_in clause(s)
        if(array_key_exists('or_where_in', $args[0])){
            $w = $args[0]['or_where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_not_in clause(s)
        if(array_key_exists('where_not_in', $args[0])){
            $w = $args[0]['where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_not_in clause(s)
        if(array_key_exists('or_where_not_in', $args[0])){
            $w = $args[0]['or_where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // like clause(s)
        if(array_key_exists('like', $args[0])){
            $l = $args[0]['like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_like clause(s)
        if(array_key_exists('or_like', $args[0])){
            $l = $args[0]['or_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // not_like clause(s)
        if(array_key_exists('not_like', $args[0])){
            $l = $args[0]['not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_not_like clause(s)
        if(array_key_exists('or_not_like', $args[0])){
            $l = $args[0]['or_not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // group_by clause(s)
        if(array_key_exists('group_by', $args[0])){
            $g = $args[0]['group_by'];
            $gCt = count($g);
            for($gg = 0; $gg < $gCt; ++$gg){
                if($g[$gg]['value']){
                    $this->db->group_by($g[$gg]['field'], $g[$gg]['value']);
                }
            }
            unset($g);      
        }
        // having clause(s)
        if(array_key_exists('having', $args[0])){
            $h = $args[0]['having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // or_having clause(s)
        if(array_key_exists('or_having', $args[0])){
            $h = $args[0]['or_having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->or_having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // join clause(s)
        if(array_key_exists('join', $args[0])){
            $j = $args[0]['join'];
            $jCt = count($j);
            for($jj = 0; $jj < $jCt; ++$jj){
                $this->db->join($j[$jj]['table'], $j[$jj]['on'], $j[$jj]['direction']);
            }
            unset($j);              
        }
        $data = array();
        $tname = '';
        $valid = TRUE;
        // array of data to insert: required
        if(array_key_exists('update', $args[0])){
            $data = $args[0]['update'];
        }else{
            $valid = FALSE;
            $this->$msg .= 'You need to specify field/value pairs of data to update.';
        }
        // table name: required
        if(array_key_exists('table_name', $args[0])){
            $tname = $args[0]['table_name'];
        }else{
            $valid = FALSE;
            $this->$msg .= 'You need to specify a table name to update.';
        }
        // setup our validations
        if(array_key_exists('validations', $args[0])){
            $v = $args[0]['validations'];
            $vCt = count($v);
            for($vv = 0; $vv < $vCt; ++$vv){
                $this->form_validation->set_rules($v[$vv]['field'], $v[$vv]['label'], $v[$vv]['validation']);
            }
            if ($this->form_validation->run() === FALSE){
                $valid = FALSE;
                $this->$msg .= validation_errors();
            }
        }
        if($valid){
            $this->db->update($tname, $data);
            $a = ($this->db->affected_rows() > 0);
            if(!$a){
                $this->$msg .= $this->db->_error_message();
                return FALSE;
            }else{
                return TRUE;
            }
        }else{
            return $valid;  
        }
    }

    // setup our delete
    private function setupDelete($args){
        // where clause(s)
        if(array_key_exists('where', $args[0])){
            $w = $args[0]['where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where clause(s)
        if(array_key_exists('or_where', $args[0])){
            $w = $args[0]['or_where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_in clause(s)
        if(array_key_exists('where_in', $args[0])){
            $w = $args[0]['where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_in clause(s)
        if(array_key_exists('or_where_in', $args[0])){
            $w = $args[0]['or_where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_not_in clause(s)
        if(array_key_exists('where_not_in', $args[0])){
            $w = $args[0]['where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_not_in clause(s)
        if(array_key_exists('or_where_not_in', $args[0])){
            $w = $args[0]['or_where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // like clause(s)
        if(array_key_exists('like', $args[0])){
            $l = $args[0]['like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_like clause(s)
        if(array_key_exists('or_like', $args[0])){
            $l = $args[0]['or_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // not_like clause(s)
        if(array_key_exists('not_like', $args[0])){
            $l = $args[0]['not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_not_like clause(s)
        if(array_key_exists('or_not_like', $args[0])){
            $l = $args[0]['or_not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // group_by clause(s)
        if(array_key_exists('group_by', $args[0])){
            $g = $args[0]['group_by'];
            $gCt = count($g);
            for($gg = 0; $gg < $gCt; ++$gg){
                if($g[$gg]['value']){
                    $this->db->group_by($g[$gg]['field'], $g[$gg]['value']);
                }
            }
            unset($g);      
        }
        // having clause(s)
        if(array_key_exists('having', $args[0])){
            $h = $args[0]['having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // or_having clause(s)
        if(array_key_exists('or_having', $args[0])){
            $h = $args[0]['or_having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->or_having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // join clause(s)
        if(array_key_exists('join', $args[0])){
            $j = $args[0]['join'];
            $jCt = count($j);
            for($jj = 0; $jj < $jCt; ++$jj){
                $this->db->join($j[$jj]['table'], $j[$jj]['on'], $j[$jj]['direction']);
            }
            unset($j);              
        }
        // table name: required
        if(array_key_exists('table_name', $args[0])){
            return $args[0]['table_name'];
        }
    }

    // setup(and run) our insert
    private function setupInsert($args){
        $data = array();
        $tname = '';
        $valid = TRUE;
        // array of data to insert: required
        if(array_key_exists('insert', $args[0])){
            $data = $args[0]['insert'];
        }else{
            $valid = FALSE;
            $this->$msg .= 'You need to specify field/value pairs of data to insert.';
        }
        // table name: required
        if(array_key_exists('table_name', $args[0])){
            $tname = $args[0]['table_name'];
        }else{
            $valid = FALSE;
            $this->$msg .= 'You need to specify a table name to insert data into.';
        }
        // setup our validations
        if(array_key_exists('validations', $args[0])){
            $v = $args[0]['validations'];
            $vCt = count($v);
            for($vv = 0; $vv < $vCt; ++$vv){
                $this->form_validation->set_rules($v[$vv]['field'], $v[$vv]['label'], $v[$vv]['validation']);
            }
            if ($this->form_validation->run() === FALSE){
                $valid = FALSE;
                $this->$msg .= validation_errors();
            }
        }
        if($valid){
            $this->db->insert($tname, $data);
            $a = ($this->db->affected_rows() > 0);
            if(!$a){
                $this->$msg .= $this->db->_error_message();
                return FALSE;
            }else{
                $this->last_id = $this->db->insert_id();
                return TRUE;
            }
        }else{
            return $valid;  
        }
    }

    // setup our select
    private function setupSelect($args){
        // select field names
        if(array_key_exists('select', $args[0])){
            $this->db->select($args[0]['select']);
        }
        // where clause(s)
        if(array_key_exists('where', $args[0])){
            $w = $args[0]['where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where clause(s)
        if(array_key_exists('or_where', $args[0])){
            $w = $args[0]['or_where'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_in clause(s)
        if(array_key_exists('where_in', $args[0])){
            $w = $args[0]['where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_in clause(s)
        if(array_key_exists('or_where_in', $args[0])){
            $w = $args[0]['or_where_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // where_not_in clause(s)
        if(array_key_exists('where_not_in', $args[0])){
            $w = $args[0]['where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // or_where_not_in clause(s)
        if(array_key_exists('or_where_not_in', $args[0])){
            $w = $args[0]['or_where_not_in'];
            $wCt = count($w);
            for($ww = 0; $ww < $wCt; ++$ww){
                if($w[$ww]['value']){
                    $this->db->or_where_not_in($w[$ww]['field'], $w[$ww]['value']);
                }
            }
            unset($w);              
        }
        // like clause(s)
        if(array_key_exists('like', $args[0])){
            $l = $args[0]['like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_like clause(s)
        if(array_key_exists('or_like', $args[0])){
            $l = $args[0]['or_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // not_like clause(s)
        if(array_key_exists('not_like', $args[0])){
            $l = $args[0]['not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // or_not_like clause(s)
        if(array_key_exists('or_not_like', $args[0])){
            $l = $args[0]['or_not_like'];
            $lCt = count($l);
            for($ll = 0; $ll < $lCt; ++$ll){
                if($l[$ll]['value']){
                    $this->db->or_not_like($l[$ll]['field'], $l[$ll]['value'], (isset($l[$ll]['wildcard'])) ? $l[$ll]['wildcard'] : null);
                }
            }
            unset($l);
        }
        // group_by clause(s)
        if(array_key_exists('group_by', $args[0])){
            $g = $args[0]['group_by'];
            $gCt = count($g);
            for($gg = 0; $gg < $gCt; ++$gg){
                if($g[$gg]['value']){
                    $this->db->group_by($g[$gg]['field'], $g[$gg]['value']);
                }
            }
            unset($g);      
        }
        // distinct flag
        if(array_key_exists('distinct', $args[0]) && $args[0]['distinct'] === TRUE){
            $this->db->distinct();
        }
        // having clause(s)
        if(array_key_exists('having', $args[0])){
            $h = $args[0]['having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // or_having clause(s)
        if(array_key_exists('or_having', $args[0])){
            $h = $args[0]['or_having'];
            $hCt = count($h);
            for($hh = 0; $hh < $hCt; ++$hh){
                if($h[$hh]['value']){
                    $this->db->or_having($h[$hh]['field'], $h[$hh]['value']);
                }
            }
            unset($h);              
        }
        // order_by clause(s)
        if(array_key_exists('order_by', $args[0])){
            $o = $args[0]['order_by'];
            $oCt = count($o);
            for($oo = 0; $oo < $oCt; ++$oo){
                if($o[$oo]['direction']){
                    $this->db->order_by($o[$oo]['field'], $o[$oo]['direction']);
                }else{
                    $this->db->order_by($o[$oo]['field']);
                }
            }
            unset($o);                  
        }
        // join clause(s)
        if(array_key_exists('join', $args[0])){
            $j = $args[0]['join'];
            $jCt = count($j);
            for($jj = 0; $jj < $jCt; ++$jj){
                $this->db->join($j[$jj]['table'], $j[$jj]['on'], $j[$jj]['direction']);
            }
            unset($j);              
        }
        // limit
        if(array_key_exists('limit', $args[0])){
            $this->db->limit($args[0]['limit']['limit'], $args[0]['limit']['offset']);
        }
        // table name: required
        if(array_key_exists('table_name', $args[0])){
            return $args[0]['table_name'];
        }
    }

}

?>
\$\endgroup\$
4
  • \$\begingroup\$ With that much code, could you add a description of what the code is doing ? It could help to know at first glance what your code is about. \$\endgroup\$
    – Marc-Andre
    Mar 24, 2014 at 18:50
  • \$\begingroup\$ making a easier to use wrapper \$\endgroup\$
    – Kevin
    Mar 24, 2014 at 19:01
  • \$\begingroup\$ please do not edit code in a already answered question. If you want to include changes made after including answers, please add a new section or ask a new question. more information and reasoning \$\endgroup\$
    – Vogel612
    Mar 25, 2014 at 15:07
  • \$\begingroup\$ Please turn on notices and warnings (error_reporting(-1)), you're calling non-static methods statically all over the place. Don't. Also, if you want code to be reviewed, please follow the (coding-style conventions)[php-fig.org] your framework subscribes to. It makes the reviewer's life easier not having to get accustomed to your indentation-style. Oh, and 10,000 records is all fine and dandy, but I'd test my code up to 10,000,000 records before making any assumptions as far as performance goes. I'd also use apache benchmark (ab -c 100 -n 10000) \$\endgroup\$ Mar 26, 2014 at 14:26

2 Answers 2

5
\$\begingroup\$

This Function can be shorter.

public function qdelete(){
    $tbl = self::prepArgs(4, func_get_args());
    $this->db->delete('Storage_Users');
    if($this->db->affected_rows() > 0){
        return TRUE;
    }else{
        $this->msg = 'There was an issue removing that record.<br />' . $this->db->_error_message();
        return FALSE;
    }
}

If you write it like this

public function qdelete(){
    $tbl = self::prepArgs(4, func_get_args());
    $this->db->delete('Storage_Users');
    if($this->db->affected_rows() <= 0){
        $this->msg = 'There was an issue removing that record.<br />' . $this->db->_error_message();
        return FALSE;
    }
    return TRUE;
}

this probably just seems like a code golf, but this is really straight to the point, and doesn't clutter your code with else statements.


This little bit of Code has some extra's in it that you don't need as well.

    if($valid){
        $this->db->update($tname, $data);
        $a = ($this->db->affected_rows() > 0);
        if(!$a){
            $this->$msg .= $this->db->_error_message();
            return FALSE;
        }else{
            return TRUE;
        }
    }else{
        return $valid;  
    }

When you enter the outside if statement, it's valid then you have the nested if statement. if !$a then it will return a message otherwise it is true. you could just write it like this

    if($valid){
        $this->db->update($tname, $data);
        if(!($this->db->affected_rows() > 0)){
            $this->$msg .= $this->db->_error_message();
            return FALSE;
        }
        return TRUE;
    } else {
        return $valid;
    }

this way you don't have to declare an extra variable, the variable isn't used for anything else here and just uses resources in being declared.


Function SetupEdits

ugh!

what is $w? Why is $w unset inside the if statement, the scope should destroy it when focus leaves the if statement. if you did need to unset these variables then you should unset the others as well $wCt

It really looks to me like you need to break these if statements into separate functions, that function is just too big and ugly looking.

talking about the if then statements in the functions

I noticed that you use them over and over again. it kind of looks like you weren't sure how to structure the functions. like whether to put action stuff together or the components together. you can actually put the components together first, in a function, and then create the functions the way you have them with the other functions being called inside of them. it will make all of them look a lot cleaner, and probably reduce some of the code.

\$\endgroup\$
6
  • \$\begingroup\$ not only is the first point arguably bad advice, you also switched two lines so that you return before you can set $this->msg. \$\endgroup\$
    – amon
    Mar 24, 2014 at 22:25
  • \$\begingroup\$ you say arguably, do you mean that some would say it is bad where some would say it is good? or would everyone have mixed feelings? @amon \$\endgroup\$
    – Malachi
    Mar 24, 2014 at 22:45
  • \$\begingroup\$ Why break them up into seperate functions? Because of the re-use of some of them? \$\endgroup\$
    – Kevin
    Mar 25, 2014 at 12:01
  • \$\begingroup\$ @o7thWebDesign, yes. I noticed that you use them over and over again. it kind of looks like you weren't sure how to structure the functions. like whether to put action stuff together or the components together. you can actually put the components together first, in a function, and then create the functions the way you have them with the other functions being called inside of them. it will make all of them look a lot cleaner, and probably reduce some of the code. \$\endgroup\$
    – Malachi
    Mar 25, 2014 at 12:35
  • \$\begingroup\$ @Malachi talking about the if then statements in the functions not sure what you mean about the paragraph under this. \$\endgroup\$
    – Kevin
    Mar 25, 2014 at 14:36
2
+50
\$\begingroup\$

By using a framework like CodeIgniter, which has a database abstraction layer, your application will benefit from model-view-controller separation and avoid SQL injection bugs. That automatically makes your application better than the average PHP code. That's a good start.


I'll lead with a trivial observation first. In PHP library code, such as this file, omit the trailing ?>. Why? It's allowable to have the opening <?php tag unmatched. It is easy to end up with a Carriage return and/or Newline at the end of the file, which the PHP interpreter will immediately print as output, since it's a templating language. Once the HTTP body starts, any subsequent code will not be able to manipulate HTTP headers: you can't issue cookies, set cache control headers, or set the HTTP status code without getting an error.

The heart of your code appears to be prepArgs($which, $args), which in turn calls setupSelect($args), setupInsert($args), setupEdit($args), or setupDelete($args) based on the value of $which. The use of 1/2/3/4 for $which had me initially puzzled: why does qdetails() need to prep 1 argument, while qinsert() needs 2 arguments, and qedit() needs 3? Instead of magic numbers, please define() some constants.

The choice of the name setupEdit() is odd. SELECT, INSERT, and DELETE are all SQL commands; EDIT is not. I could understand if you consistently used "application-layer" terminology such as "list", "add", "delete", and "edit". However, given the names of the existing three other functions, it should just be setupUpdate().

The behaviour of prepArgs(1, …) and prepArgs(4, …) is inconsistent with prepArgs(2, …) and prepArgs(3, …). With 1 and 4, it really just prepares the arguments, but does not call $this->db->get() or $this->db->delete(). With 2 and 3, it actually does call $this->db->insert() and $this->db->update(). That seems like a source of confusion and eventual bugs.

The paginator() member and $this->pagination seem to be independent of everything else in the class. I think that paginating is a responsibility of the controller, not the model. The $config that you build in the paginator() method is definitely presentation-layer code, not database-layer code.


What has me most concerned is that the bulk of setupSelect(), setupEdit(), and setupDelete() are identical repeated code. At a minimum, the common clauses 'where', 'or_where', 'where_in', 'or_where_in', 'where_not_in', 'or_where_not_in', 'like', 'or_like', 'not_like', 'or_not_like', 'group_by', 'having', 'or_having', and 'join' should be in a private helper function that all of them call.

Even after factoring out the common code into a helper function, it's still a lot of almost-repeated code! Unfortunately, PHP does not let you call member methods of $this->db dynamically using reflection, so it probably can't be reduced much further.

At this point, I have to ask: what do you gain for these 760 lines of code (including blank lines and comments)? Mostly, it seems to provide an alternate syntax for expressing your SQL clauses as arrays rather than sequential method calls. Perhaps you need that for your application, but at first glance, I'm skeptical of the cost-benefit balance.

\$\endgroup\$
1
  • \$\begingroup\$ thank you for all this. I had already refactored most of it taking the commons out and into its own private helper. Im not real concerned with naming conventions ;) this wuld b alot differner in c# ;) for the most part the paging is in there for the limits on the queries. and the point of this is to prevent havin to type $this->db stuff fo every single model i may need to write out :) \$\endgroup\$
    – Kevin
    Mar 30, 2014 at 12:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.