Controller for an Administrator User, can this be improved? (codeigniter) I want to write better code.
This is a simple controller class for an administrator login.

Are there conventions or tips in PHP to rewrite this code and improve it?

&lt;?php

class Administrators extends CI_Controller {

    public function __construct() {
        parent::__construct();
    }

    public function index() {
        if(!$this-&gt;session-&gt;userdata('logged_in')) {
            redirect('/office/administrators/login');
        } else {
            redirect('/office/dashboard/');
        } 
    }

    public function login() {

        if($this-&gt;session-&gt;userdata('logged_in')) {
            redirect('/office/dashboard/');
        } 

        if($_SERVER['REQUEST_METHOD'] == 'POST') {

             $a = new Administrator();

            if($a-&gt;login($this-&gt;input-&gt;post('email'),$this-&gt;input-&gt;post('password'))) {
                    redirect('/office/dashboard/');
            } else {
                $this-&gt;messages-&gt;add('Unable to authenticate you', 'error');

            }
        }

        $data['error_messages'] = $this-&gt;messages-&gt;get('error'); 
        $this-&gt;load-&gt;view('/office/administrators/login', $data);
    }

    public function logout() {
        $this-&gt;simpleloginsecure-&gt;logout();
        redirect('office/administrators/login');
    }

}

?&gt;
the very first characters should be &lt;? to avoid whitespace from being sent in the response. This saves you from issues with header redirects. You should also omit the closing ?&gt; for the same reason (whitespaces).

In order for this function to work it must be used before anything is outputted to the browser since it utilizes server headers.


you can use the input class's post() to check for the post request.

The function (post()) returns FALSE (boolean) if there are no items in the POST.



Here's modified code with comments:
&lt;?php
//should be the very first characters

class Administrators extends CI_Controller {

    public function __construct() {
        parent::__construct();
    }

    //factor out the login check to isolate the common logic
    //also, for understandability, boolean checks should be prefixed
    //with &quot;is&quot; or &quot;has&quot; like phrases 
    //like &quot;is it like this?&quot;, &quot;[something] has this?&quot;
    private function isLoggedIn(){
       return $this-&gt;session-&gt;userdata('logged_in');
    }

    public function index() {

        //use our new &quot;isLoggedIn()&quot;
        if(isLoggedIn()) {
            redirect('/office/dashboard/');
        } else {
            redirect('/office/administrators/login');
        } 
    }

    public function login() {

        if(isLoggedIn()) {
            redirect('/office/dashboard/');
        } 

        //you can use the input class's post() to check for the post request
        if($this-&gt;input-&gt;post()) {

            //for readability, it's more appropriate to
            //assign these values to verbose variables names...
            $administrator = new Administrator();
            $username = $this-&gt;input-&gt;post('email');
            $password = $this-&gt;input-&gt;post('password');

            //...so we know what arguments were sent
            if($administrator-&gt;login($username,$password)) {
                redirect('/office/dashboard/');
            } else {
                $this-&gt;messages-&gt;add('Unable to authenticate you', 'error');
            }
        }

        $data['error_messages'] = $this-&gt;messages-&gt;get('error'); 
        $this-&gt;load-&gt;view('/office/administrators/login', $data);
    }

    public function logout() {
        $this-&gt;simpleloginsecure-&gt;logout();
        redirect('office/administrators/login');
    }

}

//remove the closing ?&gt;.