Codeigniter 3 micro-blogging application

Question

I am working on a basic blog application in Codeigniter 3.1.8 and Bootstrap 4.

The application allows Registration (as an author), Login, adding Categories and Posts.

I have created an installation process for the application: after creating a database and providing its credentials to the application/config/database.php file, you can run the Install controller which will create all the necessary tables:

class Install extends CI_Controller {
    public function __construct()
    {
        parent::__construct();
    }

    public function index(){
        // Create all the database tables if there are none
        // by redirecting to the Migrations controller
        $tables = $this->db->list_tables();
        if (count($tables) == 0) {
            redirect('migrate');
        } else {
            redirect('/');
        }
    }
}

After that, you can register as an author. Being the first registered author, you are also an admin, meaning that your author account does not require activation (and the value for the is_admin column has a value of 1 in the database record for you).

All the future authors will need their accounts activated by you in order to publish articles (posts).

It is a rather ambitious project, although I did not start it with great plans in mind. I just wanted to learn Codeigniter 3. You can see it on my GitHub account.

There are a few possible issues:

1. The application does not use HMVC, but "classic" MVC.
2. It does not have a clear separation between its front and its back (admin)
3. I want to add a theming feature and I kinda got stuck with it; I don't know what approach to use.
4. My controllers may be a over-coded.

Here is the Posts controller:

 class Posts extends CI_Controller {

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

    private function _initPagination($path, $totalRows, $query_string_segment = 'page') {
    //load and configure pagination 
        $this->load->library('pagination');
        $config['base_url'] = base_url($path);
        $config['query_string_segment'] = $query_string_segment; 
        $config['enable_query_strings'] =TRUE;
        $config['reuse_query_string'] =TRUE;
        $config['total_rows'] = $totalRows;
        $config['per_page'] = 12;
        if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
            $_GET[$config['query_string_segment']] = 1;
        }
        $this->pagination->initialize($config);

        $limit = $config['per_page'];
        $offset = ($this->input->get($config['query_string_segment']) - 1) * $limit;

        return ['limit' => $limit, 'offset' => $offset];
    }

    public function index() {

    //call initialization method
        $config = $this->_initPagination("/", $this->Posts_model->get_num_rows());

        $data = $this->Static_model->get_static_data();
        $data['pages'] = $this->Pages_model->get_pages();
        $data['categories'] = $this->Categories_model->get_categories();  

        //use limit and offset returned by _initPaginator method
        $data['posts'] = $this->Posts_model->get_posts($config['limit'], $config['offset']);
        $this->load->view('partials/header', $data);
        $this->load->view('posts');
        $this->load->view('partials/footer');
    }

    public function search() {
    // Force validation since the form's method is GET
        $this->form_validation->set_data($this->input->get());
        $this->form_validation->set_rules('search', 'Search term', 'required|trim|min_length[3]',array('min_length' => 'The Search term must be at least 3 characters long.'));
        $this->form_validation->set_error_delimiters('<p class = "error search-error">', '</p>
            ');
        // If search fails
        if ($this->form_validation->run() === FALSE) {
            return $this->index();
        } else {
            $expression = $this->input->get('search');
            $posts_count = $this->Posts_model->search_count($expression);
            $query_string_segment = 'page';
            $config = $this->_initPagination("/posts/search", $posts_count, $query_string_segment);
            $data = $this->Static_model->get_static_data();
            $data['pages'] = $this->Pages_model->get_pages();
            $data['categories'] = $this->Categories_model->get_categories();
      //use limit and offset returned by _initPaginator method
            $data['posts'] = $this->Posts_model->search($expression, $config['limit'], $config['offset']);
            $data['expression'] = $expression;
            $data['posts_count'] = $posts_count;
            $this->load->view('partials/header', $data);
            $this->load->view('search');
            $this->load->view('partials/footer');
        }
    } 

    public function byauthor($authorid){
        $data = $this->Static_model->get_static_data();
        $data['pages'] = $this->Pages_model->get_pages();
        $data['categories'] = $this->Categories_model->get_categories(); 
        $data['posts'] = $this->Posts_model->get_posts_by_author($authorid); 
        $data['posts_count'] = $this->Posts_model->posts_by_author_count($authorid); 
        $data['posts_author'] = $this->Posts_model->posts_author($authorid);

        $this->load->view('partials/header', $data);
        $this->load->view('posts_by_author');
        $this->load->view('partials/footer');
    }

    public function post($slug) {
        $data = $this->Static_model->get_static_data();
        $data['pages'] = $this->Pages_model->get_pages();
        $data['categories'] = $this->Categories_model->get_categories();
        $data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);
        $data['post'] = $this->Posts_model->get_post($slug);

        if ($data['categories']) {
            foreach ($data['categories'] as &$category) {
                $category->posts_count = $this->Posts_model->count_posts_in_category($category->id);
            }
        }

        if (!empty($data['post'])) {
            // Overwrite the default tagline with the post title
            $data['tagline'] = $data['post']->title;

            // Get post comments
            $post_id = $data['post']->id;
            $data['comments'] = $this->Comments_model->get_comments($post_id);

            $this->load->view('partials/header', $data);
            $this->load->view('post');
        } else {
            $data['tagline'] = "Page not found";
            $this->load->view('partials/header', $data);
            $this->load->view('404');
        }
        $this->load->view('partials/footer');
    }

    public function create() {

        // Only logged in users can create posts
        if (!$this->session->userdata('is_logged_in')) {
            redirect('login');
        }

        $data = $this->Static_model->get_static_data();
        $data['pages'] = $this->Pages_model->get_pages();
        $data['tagline'] = "Add New Post";
        $data['categories'] = $this->Categories_model->get_categories();
        $data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);

        if ($data['categories']) {
            foreach ($data['categories'] as &$category) {
                $category->posts_count = $this->Posts_model->count_posts_in_category($category->id);
            }
        }

        $this->form_validation->set_rules('title', 'Title', 'required');
        $this->form_validation->set_rules('desc', 'Short description', 'required');
        $this->form_validation->set_rules('body', 'Body', 'required');
        $this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');

        if($this->form_validation->run() === FALSE){
            $this->load->view('partials/header', $data);
            $this->load->view('create-post');
            $this->load->view('partials/footer');
        } else {
            // Create slug (from title)
            $slug = url_title($this->input->post('title'), 'dash', TRUE);
            $slugcount = $this->Posts_model->slug_count($slug);
            if ($slugcount > 0) {
                $slug = $slug."-".$slugcount;
            }

            // Upload image
            $config['upload_path'] = './assets/img/posts';
            $config['allowed_types'] = 'jpg|png';
            $config['max_size'] = '2048';

            $this->load->library('upload', $config);

            if(!$this->upload->do_upload()){
                $errors = array('error' => $this->upload->display_errors());
                $post_image = 'default.jpg';
            } else {
                $data = array('upload_data' => $this->upload->data());
                $post_image = $_FILES['userfile']['name'];
            }

            $this->Posts_model->create_post($post_image, $slug);
            $this->session->set_flashdata('post_created', 'Your post has been created');
            redirect('/');
        }
    }

    public function edit($id) {
        // Only logged in users can edit posts
        if (!$this->session->userdata('is_logged_in')) {
            redirect('login');
        }

        $data = $this->Static_model->get_static_data();
        $data['pages'] = $this->Pages_model->get_pages();
        $data['categories'] = $this->Categories_model->get_categories();
        $data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);
        $data['post'] = $this->Posts_model->get_post($id);
        if ($this->session->userdata('user_id') == $data['post']->author_id) {
            $data['tagline'] = 'Edit the post "' . $data['post']->title . '"';
            $this->load->view('partials/header', $data);
            $this->load->view('edit-post');
            $this->load->view('partials/footer');
        } else {
            /* If the current user is not the author
            of the post do not alow edit */
            redirect('/' . $id);
        }
    }

    public function update() {
        // Form data validation rules
        $this->form_validation->set_rules('title', 'Title', 'required',  array('required' => 'The %s field can not be empty'));
        $this->form_validation->set_rules('desc', 'Short description', 'required',  array('required' => 'The %s field can not be empty'));
        $this->form_validation->set_rules('body', 'Body', 'required',  array('required' => 'The %s field can not be empty'));
        $this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');

        $id = $this->input->post('id');

        // Update slug (from title)
        if (!empty($this->input->post('title'))) {
            $slug = url_title($this->input->post('title'), 'dash', TRUE);
            $slugcount = $this->Posts_model->slug_count($slug);
            if ($slugcount > 0) {
                $slug = $slug."-".$slugcount;
            }
        } else {
            $slug = $this->input->post('slug');
        }

    // Upload image
        $config['upload_path'] = './assets/img/posts';
        $config['allowed_types'] = 'jpg|png';
        $config['max_size'] = '2048';

        $this->load->library('upload', $config);

        if ( isset($_FILES['userfile']['name']) && $_FILES['userfile']['name'] != null ) 
        {
            // Use name field in do_upload method
            if (!$this->upload->do_upload('userfile')) {
                $errors = array('error' => $this->upload->display_errors());

            } else {
                $data = $this->upload->data();
                $post_image = $data[ 'raw_name'].$data[ 'file_ext'];
            }
        }
        else {
            $post_image = $this->input->post('postimage');
        }


        if ($this->form_validation->run()) {
            $this->Posts_model->update_post($id, $post_image, $slug);
            $this->session->set_flashdata('post_updated', 'Your post has been updated');
            redirect('/' . $slug);
        } else {
            $this->form_validation->run();
            $this->session->set_flashdata('errors', validation_errors());
            redirect('/posts/edit/' . $slug);
        }
    }

    public function delete($slug) {
        // Only logged in users can delete posts
        if (!$this->session->userdata('is_logged_in')) {
            redirect('login');
        }

        $data['post'] = $this->Posts_model->get_post($slug);
        if ($this->session->userdata('user_id') == $data['post']->author_id) 
      {
            $this->Posts_model->delete_post($slug);
            $this->session->set_flashdata('post_deleted', 'The post has been deleted');
            redirect('/');
        } else {
            /* If the current user is not the author
            of the post do not alow delete */
            redirect('/' . $slug);
        }
      }    
    }

Please help me with useful feedback and suggestions.

Answer 1

Foreword

I haven't used CodeIgniter in the past so I am not as well-versed with the idiomatic ways it is used but I have used similar frameworks like Laravel.

I did look at the code in the github repository for references (e.g. the routes) but am mostly planning to review the code embedded directly in the post.

General Feedback

I see a fair amount of repeated code. It is wise to abstract out such repeated blocks into methods (make them private if you wish) and call those methods when appropriate. That way if a change needs to happen to each place, it can be done in one spot instead of each occurrence.

In this presentation about cleaning up code Rafael Dohms talks about limiting the indentation level to one per method and avoiding the else keyword. (see the slides here).

It is wise to return early - which your code does in a few places - like in edit() and delete() when the user isn't logged in. But there are other places this could be done. See below for more details on this.

Specific Feedback

Overriding Constructors without side-effects

I see both Install and Posts classes have a constructor that merely calls the parent method:

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

If nothing else happens in that method, there isn't really a point to override it.

The CodeIgniter Documentation for Controller has a section about _Class Constructors which states:

If you intend to use a constructor in any of your Controllers, you MUST place the following line of code in it:

parent::__construct();

That documentation could be updated to state that it only needs to be overridden if code is added beyond what the parent constructor does. While it may be very unlikely that the signature of the base constructor will never change, it is possible. If that did happen (which would likely lead to a major version release) all code using it would need to be updated. Without overriding it then it wouldn't need to be updated.

---

Else blocks

In the search() method there is a conditional block with a return statement, and then an else block:

// If search fails
    if ($this->form_validation->run() === FALSE) {
        return $this->index();
    } else {

There isn't really any need to use the else here.

The same is true in the method Install::index() there is this logic:

if (count($tables) == 0) {
        redirect('migrate');
    } else {
        redirect('/');
    }

Since redirect() terminates the script, it could be treated as an early return (just as Posts methods edit(), create() and delete() also have early returns with redirect()). Thus the else is not needed - the redirect('/') can just be the last statement in the method, after the conditional block.

A ternary operator could also be used to shorten that block to a single line:

redirect( count($tables) == 0 ? 'migrate' : '/' );

---

Repeated Code

Both the methods post() and create() have the following lines. Those could be abstracted out to a separate method and called in both places.

$data = $this->Static_model->get_static_data();
$data['pages'] = $this->Pages_model->get_pages();

$data['categories'] = $this->Categories_model->get_categories();
$data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);

if ($data['categories']) {
    foreach ($data['categories'] as &$category) {
        $category->posts_count = $this->Posts_model->count_posts_in_category($category->id);
    }
}

Those could also be abstracted into a separate method to get the data object and called in both places. Actually, I see those first four lines mentioned above are also present in the edit() method so the block to set the counts on each category could be run if categories is set and optionally when a parameter is passed to the method.

---

Calls to Posts_model::sidebar_posts()

Also on this line in create(), post() and edit():

$data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);

it is likely doing more than you think- $limit is assigned the value 5 and $offset is assigned the value 0. There isn't really anything wrong with this but if those methods created a variable with either of those names before this line then it would be over-written. Perhaps you copied the method signature from Posts_model::sidebar_posts() and added the assignment; Instead just pass the values without the assignment:

$data['posts'] = $this->Posts_model->sidebar_posts(5, 0);

---

updating page parameter in $_GET

The _initPagination() method has this block:

if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
    $_GET[$config['query_string_segment']] = 1;
}

And given that

    $config['query_string_segment'] = $query_string_segment;

and $query_string_segment isn't changed between there, it may be slightly simpler to read if the block above is changed to:

if (!isset($_GET[$query_string_segment]) || $_GET[$query_string_segment] < 1) {
    $_GET[$query_string_segment] = 1;
}