Return to Revisions

In accordance with PSR-1, your method names should be in camelCase. To avoid function name conflicts (native count() with custom count(), it may be best to prefix the method names' functionality with easy or simple, ci, or ar for Active Record.

For versatility, I recommend passing an array of arrays to your Crud methods. This will allow you to separate column names from values, enable you to turn off CI's automatic quoting (protect_identifiers flag) when necessary (3rd element in a respective subarray), and effortlessly pass multiple where conditions when required.

I mean your Read() method _could_be as simple as passing the table name and an associative array of conditions to get_where(), but that approach doesn't permit the flexibility of turning off the auto-quoting.

Make the default $condition parameter value an empty array. I don't like the meaningless WHERE 1 in any project.

Use a foreach loop to only add a WHERE clause when there is actually something useful to write inside it.

public function ciRead($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    $this->db->get($tableName)->result();
}

insert() should have the table name as the first parameter.

public function ciCreate($tableName, $data) {
    return $this->db->insert($tableName, $data) ? $this->db->insert_id() : false;
}

For update() loop the conditions like in read() I recommend returning the affected rows so that there is a truer representation of success. I often treat the affected rows value as a boolean or explicitly cast it with (bool) if I only need a truthy/falsey value and I want to make the coding intention very clear.

public function ciUpdate($tableName, $data, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->update($tableName, $data) ? $this->db->affected_rows() : false;
}

Same thinking with delete() regarding a more accurate return value...

public function ciDelete($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->delete($tableName) ? $this->db->affected_rows() : false;
}

When counting, CI has count_all_results().

public function ciCount($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->count_all_results($tableName);
}

In your controller (assuming all of the routing is set up properly), you should be collecting the slash-delimited input values as parameters of the method call instead of writing out all of those segment() calls.

Using $_GET data is perfectly fine when calling your Read() or Count() methods, but it is not recommended when you are writing to the database. For Insert(), Update(), and Delete(), you need to be collecting $_POST data only.

Be sure to validate and sanitize the incoming data in your controller before allowing access to your model methods.

Note, I wouldn't be letting people know what my table or column names are called. I wouldn't want to offer them the flexibility to nominate the table/column that they want to access as a means of security.

Perhaps provide some kind of alias/whitelist to convert the user data to model entity identifiers (translate the table/column names with in the model).

public function fetch($tableName, $conditionId) {
    $result = $this->Crud->ciRead($tableName, [['id', $conditionId]]));
    ...
}

or when writing to the db use post().

public function changeStatus() {
    $tableName = $this->input->post('tableName');
    $status = $this->input->post('status');
    $conditionId $this->input->post('conditionId');
    if ($this->Crud->ciUpdate($tableName, ['is_active' => $status], [['id', $conditionId]])) {
       $this->session->set_flashdata('success', "Success! Changes saved");
    } else {
       $this->session->set_flashdata('danger', "Something went wrong");
    }
    redirect('admin/seller/' . basename($_SERVER['HTTP_REFERER']));
}