Complete newbie, does this make sense?

[eluser]ghebert[/eluser]
I'm simply want a page where you enter a project's name (KISS for now). When it's a new project it's empty and when it's not it's prefilled with the data from the database. It's a classic case.

I'm trying to copy our current way of doing this (we have a basic custom framework). When we enter the edit page with idProject = 0, it means we're adding a new one. When it's greater than 0, we're modifying that project.

The following works, but I'm wondering if it's the "right" way of doing things:

Here's my controller:

Code:

function edit($idProject = null)
{
    // 1. when we click "add project" (/project/edit/0):     $idProject = 0
    // 2. when we click "edit project" (/project/edit/1234): $idProject = 1234
    // 3. when the filled form is posted (/project/edit):    $idProject = unspecified (null?)
    
    $this->load->model('Project_model');
    $this->load->library('validation');

    $rules['idProject'] = "numeric|trim";
    $rules['name']      = "required|trim";
    $this->validation->set_rules($rules);

    if ($idProject > 0)
    {
        $data['query'] = $this->Project_model->get_one($idProject);

        // force data in $_POST so it can be used by $this->validation->set_fields() below.
        $_POST['idProject'] = $data['query'][0]->idProject;
        $_POST['name']      = $data['query'][0]->name;
    }
    elseif ($idProject === '0')
    {
        $_POST['idProject'] = 0;
        $_POST['name']      = '';
    }


    $fields['idProject'] = $_POST['idProject'];
    $fields['name']      = $_POST['name'];
    $this->validation->set_fields($fields);

    // if we've just clicked on the "Edit project" or "Add project" button
    // or if the validation fails
    if ($idProject !== null || $this->validation->run() == FALSE)
    {
        $this->load->helper('form');
        $this->load->view('projectInfo');
    }
    else
    {
        $this->Projet_model->update_project();

        $this->load->helper('url');
        redirect('project');
    }
}

Here's my model:

Code:

class Project_model extends Model {

    var $name = '';

    function Projectmodel()
    {
        // Call the Model constructor
        parent::Model();
    }

    function get_one($idProject)
    {
        $query = $this->db->getwhere("Project", array("idProject" => $idProject));
        return $query->result();
    }

    function update_project()
    {
        $this->name = $_POST['name'];
        $data = array('name' => $this->name);

        if ($_POST['idProject'] === '0')
        {
            $this->db->insert('Project', $data);
        }
        else
        {
            $this->db->update('Project', $data, array('idProject' => $_POST['idProject']));
        }
    }

    function update_entry()
    {
        $this->name = $_POST['name'];
        $data = array('name' => $this->name);

        $this->db->update('Project', $data, array('idProject' => $_POST['idProject']));
    }
}

And the interesting lines in the view:

Code:

print $this->validation->error_string;
print form_open('project/edit', '', array('idProject' => $this->validation->idProject));
print form_input('name', $this->validation->name);
print form_submit('save', 'Save');
print form_submit('cancel', 'Cancel');
print form_close();

[eluser]Michael Wales[/eluser]
I didn't really look over your code - but this is how I would do it.

Controller:

Code:

function edit($id == FALSE) {
  $this->load->library('project_m');
  $this->load->library('validation';
  $rules['name'] = 'trim|required';
  $this->validation->set_rules($rules);
  $fields['name'] = 'Project Name';
  $this->validation->set_fields($fields);

  if ($this->validation->run()) {
    $new_id = $this->project_m->save($id, $this->input->post('name'));
    redirect('project/' . $new_id);
  } else {
    $this->data->project = $this->project_m->getProject($id);
    $this->load->view('editProject', $this->data);
  }
}

Model:

Code:

function save($id, $name) {
  if ($id === FALSE) {
    $query = $this->db->insert('projects', array('name'=>$name), array('id'=>$id));
    return $this->db->insert_id();
  } else {
    $query = $this->db->update('projects', array('name'=>$name), array('id'=>$id));
    return $id;
  }
}

function getProject($id) {
  if ($id === FALSE) {
    return FALSE;
  } else {
    $query = $this->db->getwhere('projects', array('id'=>$id), 1, 0);
    if ($query->num_rows() == 1) {
      return $query->row();
    } else {
      return FALSE;
    }
  }
}

View:

Code:

<?
  if ($project === FALSE) {
    echo form_open('controller/edit');
    $project->name = '';
  } else {
    echo form_open('controller/edit/' . $project->id);
  }
?>
<input type="text" name="name" value="<? if ($this->validation->name) { echo $this->validation->name; } else { echo $project->name; } ?>" />
<input type="submit" value="Save" />
<?= form_close(); ?>

That should work - off the top of the dome into the fast reply box. I'm sure you get the point though - you're code can be minimized a lot by assigning values to parameters if they don't exist. No need to check if $id = 0 to make a new project, just don't assign an ID and let the model determine whether it needs to insert or update (based on the default values if a parameter does not exist).

[eluser]nate_02631[/eluser]
Very elegant, walesmd. I like how you have the insert/update combined in the model as "save"... and the id check thing in the getProject model - that saves a bit of code in the controller...

I may rethink my crud method as I do this whole thing in the controller where I have to check if it's an add or edit and then either set the fields or fetch the project... but doing an id check in the model is a great idea (sometimes I'm too "literal" and stick to doing most checking in the controller)

One thing I don't understand is you have the insert/update fields hard-coded in the project model - why not pass the model an array of fieldnames/values instead? That way if you decide to add new fields later on, you don't need to update the model as well as the controller/view...


Some of my preferences:
One thing I do which I like for CRUD methods is pass an "action" as the first value of the controller uri and combine the CRUD stuff in the same method...

Code:

function project($action = '', $id = '') {

so the addresses are rather intuitive: /admin/project/edit/5, /admin/project/delete/5, /admin/project/add ... and I pass the action to the view to update the prompts on it appropriately...

I also just name the model the same as the table - "project", instead of project_model, and likewise keep the model methods simple and omit the model name from the method function name, i.e.

Code:

$this->project->get($id); vs. $this->project_model->getProject($id);

Lastly, though I see a lot of people don't do this... I try to use arrays directly whenever possible and skip assigning array values individually. I guess I find it easier to read, but maybe some ppl feel the opposite. It saves a bit of redundancy in many cases:

Code:

$this->validation->set_rules('idProject' => 'numeric|trim',
                             'name'      => 'required|trim');

(Sorry - got off on an anal-retentive coding tangent there

[eluser]Michael Wales[/eluser]
Good suggestions nate, most of which I implement within production code (particularly passing an array to the model). This example was banged out really quick just to show the code could be simplified quite a bit.

As far as the Validation arrays, I always define an array named $rules for one reason - I use a modified SetFields() method that automatically populates the $fields array. You can check out that modification here: http://ellislab.com/forums/viewreply/319879/

[eluser]ghebert[/eluser]
Thank you both. I will try this tonight.

[eluser]ghebert[/eluser]
I get the whole picture. Now there is some other stuff that I'm not sure where to put.

First, I have a Client table and a ProjectClient, which is a many-to-many. In the page where you edit the project, I want to have a checkbox for every client to indicate which one is part of the project. Should I put a new method in the Project model that would return the list of all the clients, and another method that would return all the clients for the current project?

Second, on the "index" page, where all the project are listed. I would like to put a search box and it would filter the results, I guess I can do that part alone. But I would also like to put a dropdown box where there would be all the clients listed in at least one of the projects. Selecting a client would narrow the results down to the projects where this client is in. Should I add a method in the model that would return all the clients from all the projects, and then add this to the data I'm passing to the view?

Lastly, when CI does the SQL query to insert or update a row, hoes does it know if each of the fields is an integer, a string, etc.?

Thanks!

[eluser]Michael Wales[/eluser]
1) You want clients to determine whether they are part of a project or not... odd (usually that's something an Admin does) but okay.

Make a Client model, getClients method, return a $query-result(). For each clients as client, echo out a checkbox with their name. Upon submission, the form processor will call a method on the projects model that will do clear all of the fields associating that client with that particular project, finally it will call a method that will associate the selected clients with the project in question. You have to do the clearing before you do the updating because checkboxes are not passed to the controller if unchecked. By resetting them all to the negative, then setting the ones that are passed to the positive, you ensure your checkboxes are always updated.


2) Simply make a dropdown box that when submitted goes to a some sort of status method of the project controller (or maybe the client controller, since it is the client's status on a particular project - up to you).

3) MySQL doesn't care whether it's a string/integer/etc. you are trying to pass to it. Take the following pure SQL for example:

Code:

INSERT INTO users ('id', 'email', 'created_on') VALUES ('1', '[email protected]', '2007-10-02');
UPDATE users SET 'password' = 'mypassword' WHERE 'id' = '1';

MySQL is "loosely-typed" so to speak. It only cares about field type during the creation of the table, it will then typecast any input to that fieldtype if possible (which means if you insert '1' into a INT field, it will typecast it to 1).