Welcome Guest, Not a member yet? Register   Sign In
New to CI - check out my code and tell me if what I'm doing is efficient...
#1

[eluser]johnnybravoh[/eluser]
Hello,
Below is the controller, model and view for adding an insurance agent into a database...

Since the post is too long to accommodate the model and the view, I will post them in another post...

I could easily have written this using standard procedural php but I figured I would give CI a shot. This is my first attempt. Please let me know if I'm doing anything that's not within the spirit of the MVC framework or if I could be more object oriented about things. In particular, I think that the whole address (name, address, city, state, etc....) thing is something that I've used over and over and over again. I just copy and paste code from one application to another. Working within CI, it makes me think about taking a more object oriented approach. Any thoughts???

Here's the code for your review and thanks in advance!

controller agentadmin.php
This controller is for administering an agent (in this case an insurance agent).

Code:
<?
require_once ('ApplicationController.php');
class Agentadmin extends ApplicationController {

    function Agentadmin()
    {
        parent::ApplicationController();
        $this->freakauth_light->check();
        
        //handle picture upload, if any...
        $config['relative_path']="/assets/uploads/";
        $config['upload_path'] = '.'.$config['relative_path'];
        $config['allowed_types'] = 'gif|jpg|png';
        $config['max_size']    = '100';
        $config['max_width']  = '240';
        $config['max_height']  = '360';
        $config['overwrite']  = TRUE ;
        
        $this->load->library('upload', $config);
        
        //when we first start out, this should be blank, but after a successful image upload this will contain the path to the uploaded image
        //That way if the form is submitted again due to errors, the picture is not lost...
        
        $Img="<img src=\"".$config['relative_path'].$this->session->userdata('img_name')." \" />";
        if ( !$this->upload->do_upload('picture'))
        {
            if ($Img==''){
                $this->data['picture_err']=$this->upload->display_errors('<div class="error Formerror">','</div>');
            }
            
        }    
        else
        {
            $this->data = array('upload_data' => $this->upload->data('picture'));
            $this->session->set_userdata('img_name',$this->data['upload_data']['file_name']);
            $Img="<img src=\"".$config['relative_path'].$this->session->userdata('img_name')." \" />";

        }
        $this->data['my_picture']=$Img;
        
        //handle form validation
        $this->load->library('validation');
        $this->validation->set_error_delimiters('<div class="error Formerror">', '</div>');
        $rules['name'] = "required";
        $rules['address1'] = "required";
        $rules['address2'] = "required";
        $rules['city'] = "required";
        $rules['state'] = "required";
        $rules['zip'] = "required";
        $rules['phone'] = "required";
        $rules['email'] = "required";
        $this->validation->set_rules($rules);
        
        $fields['name']    = 'name';
        $fields['address1']    = 'address1';
        $fields['address2']    = 'address2';
        $fields['city']    = 'city';
        $fields['state']    = 'state';
        $fields['zip']    = 'zip';
        $fields['phone']    = 'phone';
        $fields['email']    = 'email';
        $this->validation->set_fields($fields);
        
        $this->data['name_options']=array('name'=>'name','id'=>'name','maxlength'=>'30','size'=>'30','value'=>$this->validation->name);
        $this->data['address1_options']=array('name'=>'address1','id'=>'address1','maxlength'=>'30','size'=>'30','value'=>$this->validation->address1);
        $this->data['address2_options']=array('name'=>'address2','id'=>'address2','maxlength'=>'30','size'=>'30','value'=>$this->validation->address2);
        $this->data['city_options']=array('name'=>'city','id'=>'city','maxlength'=>'30','size'=>'30','value'=>$this->validation->city);
        $this->data['state_options']=array('name'=>'state','id'=>'state','maxlength'=>'30','size'=>'30','value'=>$this->validation->state);
        $this->data['zip_options']=array('name'=>'zip','id'=>'zip','maxlength'=>'30','size'=>'30','value'=>$this->validation->zip);
        $this->data['phone_options']=array('name'=>'phone','id'=>'phone','maxlength'=>'30','size'=>'30','value'=>$this->validation->phone);
        $this->data['email_options']=array('name'=>'email','id'=>'email','maxlength'=>'30','size'=>'30','value'=>$this->validation->email);
        $this->data['picture_options']=array('name'=>'picture','id'=>'picture','maxlength'=>'30','size'=>'30','value'=>'');

        $this->template_content['title']        = "Agents";
        $this->template_content['right_content']     = "Insurance";
    }
    function index()
    {
        $this->template_content['main_content']=$this->load->view('partials/_agentadmin_menu','',TRUE);
        $this->load->view('layouts/base', $this->template_content);
    }
    
    function add()
    {
        if ($this->validation->run() == FALSE)
        {
          $this->load->view('partials/forms/_agent_form', $this->data);
        }
        else
        {
    //post
            $this->load->model('Agent');
            $this->Agent->add();
            $this->template_content['main_content']="Agent Added Successfully!";
        }        

        $this->data['heading']     = "Add an Agent";
        
        $this->load->view('layouts/base', $this->template_content);
    
    }

}
?&gt;
#2

[eluser]johnnybravoh[/eluser]
Model Agent.php
Code:
&lt;?
class Agent extends Model {

    var $title   = '';
    var $content = '';
    var $date    = '';

    function Agent(){
    
        parent::Model();
    }


    function add(){
    
        $el=array('name','address1','address2','city','state','zip','phone','email');
        foreach ($el as $item){
            $this->item->$item=$_POST[$item];
        }
        $this->item->picture=$this->session->userdata('img_name');
        $this->db->insert('agent', $this->item);
    
    }
}
?&gt;
View _agent_form.php
Code:
&lt;!--BEGIN AGENT FORM DIV--&gt;
<div id="agentManagement">
<fieldset>
<legend>&lt;?=$heading?&gt;</legend>
    &lt;?=(isset($my_picture)?$my_picture:'')?&gt;
&lt;?=form_open_multipart($this->uri->uri_string(), array('id' => 'agent_form'))?&gt;
      <p><label for="name">Agent Name :</label>
      &lt;?=form_input($name_options)?&gt;
     &lt;?=(isset($this->validation->name_error) ? $this->validation->name_error:'');?&gt;
    </p>

      <p><label for="address1">Address 1 :</label>
      &lt;?=form_input($address1_options)?&gt;
        &lt;?=(isset($this->validation->name_error) ? $this->validation->address1_error : '')?&gt;
      </p>
    <p><label for="address2">Address 2 :</label>
    &lt;?=form_input($address2_options)?&gt;
        &lt;?=(isset($this->validation->address2_error) ? $this->validation->address2_error : '')?&gt;
    </p>
      <p><label for="city">City :</label>
      &lt;?=form_input($city_options)?&gt;
    &lt;?=(isset($this->validation->city_error) ? $this->validation->city_error : '')?&gt;
      </p>
      <p><label for="state">State :</label>
      &lt;?=form_input($state_options)?&gt;
    &lt;?=(isset($this->validation->state_error) ? $this->validation->state_error : '')?&gt;
      </p>
      <p><label for="zip">Zip :</label>
      &lt;?=form_input($zip_options)?&gt;
    &lt;?=(isset($this->validation->zip_error) ? $this->validation->zip_error : '')?&gt;
      </p>
      <p><label for="email">Email :</label>
      &lt;?=form_input($email_options)?&gt;
    &lt;?=(isset($this->validation->email_error) ? $this->validation->email_error : '')?&gt;
      </p>
      <p><label for="phone">Phone :</label>
      &lt;?=form_input($phone_options)?&gt;
    &lt;?=(isset($this->validation->phone_error) ? $this->validation->phone_error : '')?&gt;
      </p>
      <p><label for="picture">Picture :</label>
      &lt;?=form_upload($picture_options)?&gt;
    &lt;?=(isset($picture_err) ? $picture_err: '')?&gt;
      </p>
            &lt;input type="submit" name="Submit" value="Submit" class="submit"/&gt;
            &lt;input type="reset" name="Reset" value="Reset" /&gt;
&lt;/form&gt;
</fieldset>
</div>&lt;!--END AGENT FORM DIV--&gt;
#3

[eluser]tonanbarbarian[/eluser]
Here are a couple of things I see as minor issues

1. you are extending ApplicationController
Instead if you create a class called MY_Controller and put it in the libraries folder it will be included automatically. You will not need to require_once ('ApplicationController.php');

2. In your controller constructor you are putting lots of configuration for things like upload and validation.
Personally I do not put that in the constructor. The problem with doing this is all of that code is being loaded, instantiated and executed on every method in that controller whether it is needed or not.
If you do feel you need the code in more than one place put the code into seperate private methods
i.e.
Code:
function _loadUpload() {
//handle picture upload, if any...
        $config['relative_path']="/assets/uploads/";
        $config['upload_path'] = '.'.$config['relative_path'];
        $config['allowed_types'] = 'gif|jpg|png';
        $config['max_size']    = '100';
        $config['max_width']  = '240';
        $config['max_height']  = '360';
        $config['overwrite']  = TRUE ;
        
        $this->load->library('upload', $config);
        
        //when we first start out, this should be blank, but after a successful image upload this will contain the path to the uploaded image
        //That way if the form is submitted again due to errors, the picture is not lost...
        
        $Img="<img src=\"".$config['relative_path'].$this->session->userdata('img_name')." \" />";
        if ( !$this->upload->do_upload('picture'))
        {
            if ($Img==''){
                $this->data['picture_err']=$this->upload->display_errors('<div class="error Formerror">','</div>');
            }
            
        }    
        else
        {
            $this->data = array('upload_data' => $this->upload->data('picture'));
            $this->session->set_userdata('img_name',$this->data['upload_data']['file_name']);
            $Img="<img src=\"".$config['relative_path'].$this->session->userdata('img_name')." \" />";

        }
        $this->data['my_picture']=$Img;
} // _loadUpload()


function _loadValidation() {
//handle form validation
        $this->load->library('validation');
        $this->validation->set_error_delimiters('<div class="error Formerror">', '</div>');
        $rules['name'] = "required";
        $rules['address1'] = "required";
        $rules['address2'] = "required";
        $rules['city'] = "required";
        $rules['state'] = "required";
        $rules['zip'] = "required";
        $rules['phone'] = "required";
        $rules['email'] = "required";
        $this->validation->set_rules($rules);
        
        $fields['name']    = 'name';
        $fields['address1']    = 'address1';
        $fields['address2']    = 'address2';
        $fields['city']    = 'city';
        $fields['state']    = 'state';
        $fields['zip']    = 'zip';
        $fields['phone']    = 'phone';
        $fields['email']    = 'email';
        $this->validation->set_fields($fields);
} // _loadValidation()

function _loadOptions() {
$this->data['name_options']=array('name'=>'name','id'=>'name','maxlength'=>'30','size'=>'30','value'=>$this->validation->name);
        $this->data['address1_options']=array('name'=>'address1','id'=>'address1','maxlength'=>'30','size'=>'30','value'=>$this->validation->address1);
        $this->data['address2_options']=array('name'=>'address2','id'=>'address2','maxlength'=>'30','size'=>'30','value'=>$this->validation->address2);
        $this->data['city_options']=array('name'=>'city','id'=>'city','maxlength'=>'30','size'=>'30','value'=>$this->validation->city);
        $this->data['state_options']=array('name'=>'state','id'=>'state','maxlength'=>'30','size'=>'30','value'=>$this->validation->state);
        $this->data['zip_options']=array('name'=>'zip','id'=>'zip','maxlength'=>'30','size'=>'30','value'=>$this->validation->zip);
        $this->data['phone_options']=array('name'=>'phone','id'=>'phone','maxlength'=>'30','size'=>'30','value'=>$this->validation->phone);
        $this->data['email_options']=array('name'=>'email','id'=>'email','maxlength'=>'30','size'=>'30','value'=>$this->validation->email);
        $this->data['picture_options']=array('name'=>'picture','id'=>'picture','maxlength'=>'30','size'=>'30','value'=>'');

} // _loadOptions()
And so then in add() or wherever else the code is actually needed just call
Code:
$this->_loadUpload();
or
$this->_loadValidation();
or
$this->_loadOptions();

3. You might want consider having your Agent::add() method return a boolean (TRUE/FALSE) indicating whether the add was successful and then have your controller display accordingly.

4. this last one is so minor you do not really need to worry about it, but personally i never use PHP short tags &lt;? ... ?&gt;. I always use the full tags &lt;?php ... ?&gt;. In particular this means converting all &lt;?=...?&gt; to &lt;?php echo ... ?&gt;.
The reason I say this is because the short tags, while enabled by default in php is able to be turned off. And some hosting providers and server administrators will turn it off for whatever reason.
I always code with it turned off so I can be sure that any code I write will work on every PHP setup.
But if your site is always going to be run on your own server that you control then do not worry about it.
#4

[eluser]johnnybravoh[/eluser]
Thank you for your insight. You're right - that's a great idea and one that I have already implemented.

Regarding the short tags, I just always use them & have never had a problem. I'm far too lazy to type &lt;?php.... If something does happen at a hosting provider that I use I can always run a perl script on the files to quickly implement long, laborious php tags. It hasn't been a problem for me in the 5 years I've been coding php so I don't worry about it much...

Cheers!




Theme © iAndrew 2016 - Forum software by © MyBB