Reputation: 23297
I've been using codeIgniter for 5 months now and I want to improve the way I write my code. Here's a method from a controller for users:
function create_user(){
$user_data = array(
'username' => 'Username', 'firstname' => 'Firstname',
'middlename' => 'Middlename', 'lastname' => 'Lastname',
'password' => 'Password', 'department' => 'Department',
'role' => 'Role'
);
foreach ($user_data as $key => $value) {
$this->form_validation->set_rules($key, $value, 'required|trim');
}
if ($this->form_validation->run() == FALSE) {
$departments = $this->user_model->list_departments();
$it_roles = $this->user_model->list_roles(1);
$tc_roles = $this->user_model->list_roles(2);
$assessor_roles = $this->user_model->list_roles(3);
$data['data'] = array('departments' => $departments, 'it_roles' => $it_roles, 'tc_roles' => $tc_roles, 'assessor_roles' => $assessor_roles);
$data['content'] = 'admin/create_user';
parent::error_alert();
$this->load->view($this->_at, $data);
} else {
$username = $this->input->post('username');
$salt = $this->bcrypt->getSalt();
$hashed_password = $this->bcrypt->hash($this->input->post('password'), $salt);
$fname = $this->input->post('firstname');
$mname = $this->input->post('middlename');
$lname = $this->input->post('lastname');
$department = $this->input->post('department');
$role = $this->input->post('role');
$user_login = array($username, $hashed_password, $salt);
$user_profile = array($fname, $mname, $lname);
$this->user_model->register_user($user_login, $user_profile, $department, $role);
$data['content'] = 'admin/view_user';
parent::success_alert(4, 'User Sucessfully Registered!', 'You may now login using your account');
$data['data'] = array('username' => $username, 'fname' => $fname, 'mname' => $mname, 'lname' => $lname, 'department' => $department, 'role' => $role);
$this->load->view($this->_at, $data);
}
}
I basically cram in fetching of input, input validation, rendering views for errors or success, and other stuff in a single controller. What's the better way of doing this, how do you suggest I breakdown this function. Someone said that I should add a router/dispatcher between the view and the controller to handle POST
, GET
, COOKIE
and SESSION
but I don't have any idea how to do that.
Any code samples, suggestions that would help me grasp the proper way of doing things will be accepted as an answer.
Upvotes: 0
Views: 178
Reputation: 1907
I did some minor changes: Not setting unnecessary variables, grouping the userdata in an array etc. That's just the way I would do it. Other than that you're good on your way, I'd say.
<?php
function create_user()
{
$user_data = array ('username' => 'Username', 'firstname' => 'Firstname', 'middlename' => 'Middlename', 'lastname' => 'Lastname', 'password' => 'Password', 'department' => 'Department', 'role' => 'Role');
foreach ($user_data as $key => $value) {
$this->form_validation->set_rules($key, $value, 'required|trim');
}
if ($this->form_validation->run() == FALSE)
{
$data['data'] = array( 'departments' => $this->user_model->list_departments(),
'it_roles' => $this->user_model->list_roles(1),
'tc_roles' => $this->user_model->list_roles(2),
'assessor_roles'=> this->user_model->list_roles(3)
);
$data['content'] = 'admin/create_user';
parent::error_alert();
$this->load->view($this->_at, $data);
}
else
{
$salt = $this->bcrypt->getSalt();
$user['username'] = $this->input->post('username');
$user['hashed_password']= $this->bcrypt->hash($this->input->post('password'), $salt);
$user['fname'] = $this->input->post('firstname');
$user['mname'] = $this->input->post('middlename');
$user['lname'] = $this->input->post('lastname');
$user['department'] = $this->input->post('department');
$user['role'] = $this->input->post('role');
$user_login = array($user['username'], $user['hashed_password'], $salt);
$user_profile = array($user['fname'], $user['mname'], $user['lname']);
$this->user_model->register_user($user_login, $user_profile, $department, $role);
$data['content'] = 'admin/view_user';
parent::success_alert(4, 'User Sucessfully Registered!', 'You may now login using your account');
unset($user['hashed_password']); // Just to be sure
$this->load->view($this->_at, $user);
}
}
Upvotes: 1
Reputation: 140210
You'll also want to use redirection after a POST request so that you don't get duplicate submits.
function create_user() {
$params = $this->input->post();
if( !$this->user_model->valid_user_params( $params, $errors ) )
{
//Place errors in flash data
//redirect and exit
}
if( !$this->user_model->register_user($params, $errors) )
{
//Place errors in flash data
//redirect and exit
}
//redirect and exit
}
function created_user() {
//read flash data
//show some views with the errors or success message
}
Upvotes: 2