CodeIgniter Model I built

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)

```
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 ar

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.' . $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() msg = 'There was an issue removing that record.' . $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.

StackExchange Code Review Q#45090, answer score: 5