1
\$\begingroup\$

I am working on a online newspaper/blogging application with CodeIgniter 3.1.8 and Bootstrap 4. I have decided to add themes to it. The application is not HMVC, only MVC.

I thought it was a good idea to use the Twig template engine to the theme(s). For this purpose, I use CodeIgniter Simple and Secure Twig.

The theme's templates, placed in application\views\themes\caminar (caminar is the current theme's name) have the extension .twig, not .php.

The layout.twig file contains the "master layout code":

<!DOCTYPE html>
<html>
    <head>
        <title>{{site_title}} | {{tagline}}</title>
        <meta charset="utf-8" />
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <link rel="stylesheet" href="{{maincss}}">
    </head>
    <body>

        <!-- Header -->
        <header id="header">
            <div class="logo"><a href="/">{{site_title}}</a></div>
        </header>

        <section id="main">
            <div class="inner">
                {% if singlePost is defined %}
                    {{include(singlePost)}}
                {% elseif pageTemplate is defined  %}
                    {{include(pageTemplate)}}
                {% elseif notFound is defined  %}
                    {{include(notFound)}}
                {% else %}
                    {{ include('themes/caminar/templates/posts.twig') }}                
                {% endif %}
            </div>
        </section>

        <!-- Footer -->
        <footer id="footer">
            <div class="container">
                <ul class="icons">
                    <li><a href="{{twitter}}" target="_blank" class="icon fa-twitter"><span class="label">Twitter</span></a></li>
                    <li><a href="{{facebook}}" target="_blank" class="icon fa-facebook"><span class="label">Facebook</span></a></li>
                    <li><a href="{{instagram}}" target="_blank" class="icon fa-instagram"><span class="label">Instagram</span></a></li>
                    <li><a href="mailto:{{company_email}}" class="icon fa-envelope-o"><span class="label">Email</span></a></li>
                </ul>

                {% if pages %}
                    <ul class="icons">
                        {% for page in pages %}
                        <li><a href="{{base_url}}/pages/page/{{page.id}}">{{page.title}}</a></li>
                        {% endfor %}
                    </ul>
                {% endif %}
            </div>
            <div class="copyright">
                &copy; {{company_name}}. All rights reserved. Design by <a href="https://templated.co" target="_blank">TEMPLATED</a>
            </div>
        </footer>
    </body>
</html>

The posts.twig file displays the posts:

<section class="wrapper style1">
    {% for post in posts %}
        {% if loop.first %}
            <div class="image fit flush">
                <a href="{{base_url}}{{post.slug}}">
                    <img src="{{base_url}}/assets/img/posts/{{post.post_image}}" alt="{{post.title}}" />
                </a>
            </div>
            <header class="special">
                <h2>{{post.title}}</h2>
                <p>{{post.description}}</p>
            </header>
            <div class="content">{{post.content|raw}}</div>
        {% else %}
            <div class="spotlight {% if (loop.index is even) %}alt{% endif %}">
                <div class="image flush">
                    <a href="{{base_url}}{{post.slug}}">
                        <img src="{{base_url}}/assets/img/posts/{{post.post_image}}" alt="{{post.title}}" />
                    </a>
                </div>
                <div class="inner">
                    <h3>{{post.title}}</h3>
                    <p>{{post.description}}</p>
                    <div class="{% if (loop.index is even) %}text-right{% else %}text-left{% endif %}">
                        <a href="{{base_url}}{{post.slug}}" class="button special small">Read More</a>
                    </div>
                </div>
            </div>
        {% endif %}
    {% endfor %}
</section>

<div class="pagination-container">{{ pagination|raw }}</div>

Finally, the posts controller is the main reason for my concerns regarding the quality of the code:

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['base_url'] = base_url("/");
        $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->twig->addGlobal('maincss', base_url('themes/caminar/assets/css/main.css'));
        $this->twig->addGlobal('pagination', $this->pagination->create_links());
        $this->twig->display('themes/caminar/layout', $data);
    }


    public function byauthor($authorid){
        //load and configure pagination 
        $this->load->library('pagination');
        $config['base_url'] = base_url('/posts/byauthor/' . $authorid);
        $config['query_string_segment'] = 'page';
        $config['total_rows'] = $this->Posts_model->posts_by_author_count($authorid);
        $config['per_page'] = 12;
        
        if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
            $_GET[$config['query_string_segment']] = 1;
        }
        
        $limit = $config['per_page'];
        $offset = ($this->input->get($config['query_string_segment']) - 1) * $limit;
        $this->pagination->initialize($config);

        $data = $this->Static_model->get_static_data();
        $data['base_url'] = base_url("/");
        $data['pages'] = $this->Pages_model->get_pages();
        $data['categories'] = $this->Categories_model->get_categories(); 
        $data['posts'] = $this->Posts_model->get_posts_by_author($authorid, $limit, $offset); 
        $data['posts_count'] = $this->Posts_model->posts_by_author_count($authorid); 
        $data['posts_author'] = $this->Posts_model->posts_author($authorid);

        $this->twig->addGlobal('maincss', base_url('themes/caminar/assets/css/main.css'));
        $this->twig->addGlobal('pagination', $this->pagination->create_links());
        $this->twig->display('themes/caminar/layout', $data);
    }

    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['authors'] = $this->Usermodel->getAuthors();
        $data['posts'] = $this->Posts_model->sidebar_posts($limit=5, $offset=0);
        $data['post'] = $this->Posts_model->get_post($slug);
        $data['author_image'] = isset($data['post']->avatar) && $data['post']->avatar !== '' ? $data['post']->avatar : 'default-avatar.png';

        //CSS, JS and other resources add to twig here, because PHP and Codeigniter functions are not available from Twig templates
        $this->twig->addGlobal('maincss', base_url('themes/caminar/assets/css/main.css'));

        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->twig->addGlobal('singlePost','themes/caminar/templates/singlepost.twig');
            $this->twig->display('themes/caminar/layout', $data);
        } else {
                $data['tagline'] = "Page not found";
                $this->twig->addGlobal('notFound','themes/caminar/templates/404.twig');
                $this->twig->display('themes/caminar/layout', $data);

        }
    }
}

Concerns:

  1. Is the code (too) repetitive?
  2. Is the code over engineered?
\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

Since your concern is chiefly falling on the controller and I don't have particular expertise with twig scripting, I'll speak on the controller. Please do not take my snippet at the end to be ready for copy-pasta -- it is not tested and it is likely that I have made mistakes and/or misunderstood the data that is being passed into these controllers. Hopefully the itemized advice will help you to realize some techniques to clean up your methods.

  1. PSR-12 says: Method names MUST NOT be prefixed with a single underscore to indicate protected or private visibility. That is, an underscore prefix explicitly has no meaning.

  2. I prefer to declare data types on all incoming parameters and return values. Not only does this help with debugging, it also coerces values like numeric strings to ints and floats.

  3. Rather than repeat the $config each time you want to declare a new element, just declare the full array in one go.

  4. Better yet, don't declare the $config variable at all (or any single-use variables for that matter). Any time you declare a variable then only actually use it one time, then this is a good argument for writing the value directly into where you originally had the variable. There are some exceptions to this, for instance, you want to be declarative about the value or, perhaps, directly injecting the value makes the code less readable / too wide.

  5. We shouldn't be seeing any instances of $_GET in your CI code. CI passes all this slash-delimited data in the url and all of the data is instantly available in your controller method's argument list. From memory, I don't think I ever use $this->input->get() in any of my CI work.

  6. I prefer to use camelCase variable naming because PHPStorm does a good job of finding my misspellings and it is more concise than snake_case. Well, actually, $queryStringSegment is too verbose and only vaguely describes the value, $segment is just as vague, but more concise. Perhaps there is a better variable name to use.

  7. You may have confused me with /pages/page/{{page.id}}, but I am assuming that index() method is what is receiving these page load requests and the only value that you actually need is the last one {{page.id}}. Add $pageNumber as the lone parameter of index() and pass the value along with other expected values to _initPagination(). Ensure that $pageNumber cannot be less than 1 -- I'll recommend max().


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

    private function _initPagination(string $baseUrl, int $totalRows, string $segment = 'page', ?int $pageNumber = 1): array
    {
        $this->load->library('pagination');
        $limit = 12;

        $this->pagination->initialize([
            'base_url' => $baseUrl,
            'query_string_segment' => $segment,
            'enable_query_strings' => true,
            'reuse_query_string' => true,
            'total_rows' = $totalRows,
            'per_page' => $limit,
        ]);

        return [$limit, $pageNumber - 1];
    }

    public function index(int $pageNumber = 1): void
    {    
        $baseUrl = base_url('/');

        //call initialization method
        $rangeParameters = $this->_initPagination(
            $baseUrl,
            $this->Posts_model->get_num_rows(),
            'page',
            max(1, $pageNumber),
        );

        $data = $this->Static_model->get_static_data()
              + [
                    'base_url' => $baseUrl,
                    'pages' => $this->Pages_model->get_pages(),
                    'categories' => $this->Categories_model->get_categories(),
                    'posts' = $this->Posts_model->get_posts(...$rangeParameters)
                ];

        $this->twig->addGlobal('maincss', base_url('themes/caminar/assets/css/main.css'));
        $this->twig->addGlobal('pagination', $this->pagination->create_links());
        $this->twig->display('themes/caminar/layout', $data);
    }
    ...
}
\$\endgroup\$
9
  • \$\begingroup\$ Very professional. Many thanks! \$\endgroup\$ Commented Nov 25, 2020 at 11:10
  • \$\begingroup\$ Please consider giving an answer to this question. Thanks! \$\endgroup\$ Commented Nov 25, 2020 at 11:13
  • \$\begingroup\$ @mickmackusa Regarding Point 6. PhpStorm (as with most IDE's) does a fine job of finding spelling mistakes regardless of which "case" style you prefer. Which means, it works just as well with snake_case as it does with camelCase etc. I understand that is your own preference but that particular reason may be misleading to some folks. I use snake_case / camelCase where applicable, due to coding/style conventions being used, under PhpStorm and it plays nice with both. \$\endgroup\$ Commented Nov 25, 2020 at 12:48
  • \$\begingroup\$ My point is that although camelCase smashes letters together, the IDE is smart enough to understanding/catch spelling mistakes, so I recommend the more concise variable naming convention. I have a habit of not using snake_case anywhere in my projects. This also gives an obvious separation between my entity names and native function calls. I am not misleading anyone; I said "I prefer". \$\endgroup\$ Commented Nov 25, 2020 at 12:55
  • 1
    \$\begingroup\$ @Raz Just so you know, I am still interested in figuring out your bug with POST'ing from the form to the controller via ajax. I think there is a vital detail that I am not being shown. Please show me (via 3v4l.org link or similar) 1. the exact generated source code of the form that is doing the post 2. The Network > XHR > Response tab text from the Comments/create controller that shows exit(var_export(['stream' => $this->input->raw_input_stream, 'post' => $this->input->post(), 'slug' => $post_slug], true)); so that I can see that the controller is working. \$\endgroup\$ Commented Nov 26, 2020 at 22:50
1
\$\begingroup\$

Well just some refactoring.

i.e. $this->twig->display('themes/caminar/layout', $data); only needs to appear once.

When you have code that repeats inside an if/else, it can come outside of that block.

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->twig->addGlobal('singlePost','themes/caminar/templates/singlepost.twig');
    $this->twig->display('themes/caminar/layout', $data);
} else {
        $data['tagline'] = "Page not found";
        $this->twig->addGlobal('notFound','themes/caminar/templates/404.twig');
        $this->twig->display('themes/caminar/layout', $data);
}

Becomes

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->twig->addGlobal('singlePost','themes/caminar/templates/singlepost.twig');
} else {
        $data['tagline'] = "Page not found";
        $this->twig->addGlobal('notFound','themes/caminar/templates/404.twig');
}
$this->twig->display('themes/caminar/layout', $data);
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.