Codeigniter Login Controller

Instead of creating an Admin_Controller or MY_Controller I was going to try and just try out all my controllers on requirements that are needed per controller. I know this may seem like additional work but then after which I want to go back and fix it all. I wanted to know how my login looks as of right now?

I'm getting shocked that I've had 56 view and 3 upvotes and not one comment on the code at least.

```
load -> model('user_login_model', 'login');
$user_id = $this->session->userdata('user_id');
if ($user_id == TRUE)
{
if (is_numeric($user_id) && strlen($user_id) session->unset_userdata('user_id');
$this->session->sess_destroy();
current_url();
}
else
{
redirect('dashboard', 'refresh');
}
}
}

public function index()
{
$this -> template -> set_theme('saturn') -> set_layout(FALSE) -> build('admin/login');
}

public function process()
{
$output_array = $this -> general_functions -> get_default_output();
$this -> form_validation -> set_rules('email_address', 'Email Address', 'trim|required|xss_clean|valid_email');
$this -> form_validation -> set_rules('password', 'Password', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');
$this -> form_validation -> set_rules('remember_me', 'Remember Me', 'trim|xss_clean|integer');
if ($this -> form_validation -> run() == FALSE)
{
$output_array['data_array']['title'] = 'Form Validation Failed';
$output_array['data_array']['message'] = 'The form failed to validate. Please fix the errors and try again.';
}
else
{
$user_login_data = $this -> login -> get_by('email_address', $this -> input -> post('email_address'));
if (is_array($user_login_data) && count($user_login_data) user_status_id == 0)

-
Formatting is not consistent:

$this -> load -> model('user_login_model', 'login');
$user_id = $this->session->userdata('user_id');

Sometimes there are spaces around ->, sometimes aren't.

-
You could use a guard clause to make the code flatten:

$user_id = $this->session->userdata('user_id');
if ($user_id != TRUE)
{
    return;
}
if (is_numeric($user_id) && strlen($user_id) session->unset_userdata('user_id');
    $this->session->sess_destroy();
    current_url();
}
else
{
    redirect('dashboard', 'refresh');              
}

It would help a lot to make it easier to read the process function. (You should extract a few functions there too.)

-

current_url();

It's not clear what does this function do. Does it get the current url? Log it? Modify it? Its name should contain an action, like doSomething.

-
Instead of magic numbers like 0, 900, 901 etc. you should use named constants which describe their purpose. It would be easier to read.

-

$this -> general_functions -> publish_output($output_array['status'], $output_array['data_array'], NULL, NULL, 'json', TRUE);

I would create local variables for the two NULL values and the TRUE value too to explain their intent. It would make the code readable and don't force readers to check the parameters of the publish_output function.

(Clean Code by Robert C. Martin, G19: Use Explanatory Variables; Refactoring: Improving the Design of Existing Code by Martin Fowler, Introduce Explaining Variable)

StackExchange Code Review Q#41127, answer score: 4