Reduce repetition when sharing data across an application

Question

I have a CodeIgniter 3 application. There is some data every page across the application needs to have.

This is the system I currently have.

Create a model function that will get the data I need

Class General_model extends CI_Model
{
    function data_init()
    {
        $data['session'] = $this->session->userdata('session');
        $data['foobar'] = $this->general_model->get_foobar();
        return $data;
    }
}

In every controller, I call this data in the __construct()

function __construct()
{
    parent::__construct();
    $this->load->model('general_model', '', TRUE);
    $this->data = $this->general_model->data_init();
}

I begin every controller function by getting the data in the scope of the function, which then gets passed to the view

public function index()
{
    $data = $this->data;
    $data['page_title'] = 'example';
    $this->load->view('templates/header', $data);
    $this->load->view('page/example', $data);
    $this->load->view('templates/footer', $data);
}

I feel this is not ideal. It feels repetitive and not dry to have $this->data = $this->general_model->data_init(); in every constructor and $data = $this->data; in every function.

It feels like it'd be better if I could declare $this->data = $this->general_model->data_init(); once in the application and have $this->data available to every function in every controllers. I'd also like to avoid $data = $this->data; at the start of every function, but I'd rather do that than have the additional 7 characters $this-> hundreds of times across the codebase (perhaps that is me being lazy?).

I'm not sure how to improve either of those without breaking OOP and going global, or touching CodeIgniter core classes such as CI_Model.

Is there a way to improve how I'm sharing and moving this data?

Answer 1

I assume that $this->data is a property of a controller class defined like this.

class Some_controller extends CI_Controller
{
   protected $data;  //could be public or private too, not relevant to the discussion

  function __construct()
  {
     //assign a value to the $data property
     $this->data = $this->general_model->data_init();
  } 

If so, then the points about memory utilization in Mike's answer could be realized simply by using $this->data directly in each controller method. Making a copy (i.e. $data = $this->data;) is pointless because the property is already available to all controller methods. In other words, simply do this in any class method.

$this->data['page_title'] = 'example';

Then pass the class property ($data) to the view load like this.

$this->load->view('templates/header', $this->data);

In the revised index() below I have done a couple other things that probably amount to "micro-optimizations" but are worth considering. First, notice I only pass $this->data to the first load->view() call.

The documentation doesn't do a great job of pointing out that any data passed via load->view() is cached and will be available to any subsequent views loaded. If you continually pass exactly the same data you are running many extra lines of code that simply overwrite the cached data with the exact same values. If you don't pass any data this pointless exercise is eliminated.

The second "micro-optimization" is the use of method chaining. load->view() returns a CI_Loader instance which is exactly the same memory location that $this->load returns. Why make it look up that value again? And why do all the extra typing?

public function index()
{
  $this->data['page_title'] = 'example';
  $this->load
    ->view('templates/header', $this->data)
    ->view('page/example')
    ->view('templates/footer');
}

Now, with all that said, the thing that will help you the most with being DRY is to create a "base controller" that will extend CI_Controller. This "base controller" is then used to create all the other controllers for the application. Refer to the CI documentation about Extending Native Libraries for details but the basic idea starts with creating the "base controller".

Create the file application/core/MY_Controller.php

class MY_Controller extends CI_Controller
{
    protected $data;  

    public function __construct()
    {
      parent::__construct();
      $this->load->model('general_model', '', TRUE);
      $this->data = $this->general_model->data_init();
    }

The property $data will be available in any class that extends MY_Controller so use it as the base class for all other application controllers.

Since you appear to use the same set of header and footer views all the time you can also DRY out view loading.

MY_Controller continues with:

  /**
   * A template used for every page load
   * @param string $view is the name of the view file to load
   * @param array $view_data is data to be passed to view files. 
   * If no $view_data is passed then the class property $data is used
   */
  public function render_page($view, $view_data = NULL)
  {
    //Uses a ternary to decide what "data" gets sent to view
    $this->load
      ->view('templates/header', isset($view_data) ? $view_data : $this->data)
      ->view($view)
      ->view('templates/footer');
  }

} //end of MY_Controller

The following uses the CI "Welcome" example modified by extending with MY_Controller. It adds an "other_page" too.

This is the file application/controllers/Welcome.php

class Welcome extends MY_Controller
{
  public function __construct()
  {
    parent::__construct();
    //If desired you could set a default title for pages of this controller
    $this->data["page_title"] = "Welcome";
  }

  public function index()
  {
    $this->render_page('welcome_message');
  }

  public function other_page()
  {
    //overwrite the default page title
    $this->data["page_title"] = "Other Page";
    $this->render_page('other_view');
  }

}

Hopefully it is clear how this achieves the goals described in your question.

A couple last comments on what seems to be unnecessary work.

First, it appears you have set some session data that might look something like this. (Obviously, I'm making up keys and values.)

$_SESSION['session'] = array('id`=>$id, 'name'=>'Guy', 'logged_in'=>TRUE);

Putting an array in an array seems to me to be making more work than needed. Just set each item as a key in $_SESSION directly.

$_SESSION['some_key'] = $some_value;

If you look at the source code for set_userdata() the above is essentially what you will find. IMO, the best reason to use set_userdata() is to set several items at once.

$this->session->set_userdata(array('id'=>$id, 'name'=>'Guy', 'logged_in'=> TRUE));

That line produces exactly the same thing as these lines.

$_SESSION['id'] = $id;
$_SESSION['name'] = 'Guy';
$_SESSION['logged_in'] = TRUE;

The second 'final comment' is about making a copy of session data in your model with this call.

$data['session'] = $this->session->userdata('session');

Clearly you have the session library running and it contains previously set user data.

The session class can be used directly in every controller method so why make a copy of its data? Making a copy uses more memory and forces you to dereference the copy in order to utilize it - for example:

$user_name = $this->data['session']['user_name']; 

Why do something like that when, if you set the session data as suggested earlier, you could use any of the following in any controller method?

$user_name = $_SESSION['user_name']; //runs the risk of an "Undefined index" error

or

$user_name = $this->session->user_name; //using the "magic" getter - my prefered way to go

or

$user_name =  $this->session->userdata('user_name'); // the "traditional" CI way

Answer 2

I would not get all worked up about having to type a few extra letters when referencing a variable. I could get worked up over possibly doubling the storage space required for your data depending on it's size. Let's walk though what PHP is doing behind the scenes here to understand:

// PHP references $this->data to $data by reference
$data = $this->data;

// since you modify $data here, PHP now copies the entirety of $this->data
// to $data and breaks reference
// you are now taking up double (plus some) storage for your data 
$data['page_title'] = 'example';

As opposed to directly modifying $this->data before making assignment to $data like shown below , which would not double memory utilization:

$this->data['page_title'] = 'example';
$data = $this->data;

Just remember that as soon as you modify an array assigned in this manner, you remove the ability for PHP to perform it's behind the scenes memory optimization. But really you should not be relying on this anyway, as IMO it is bad coding form to be relying on some hidden, undocumented and possibly subject to change optimization for your code to be working in the best manner.

Alternately, instead of associative array for data, why not use an object (even stdClass), then you are always working with reference to the object (unless you specifically clone it)? For example:

// assuming $this->data holds object
// $data is just reference to $this->data here
$data = $this->data;

// here modifying $data would modify both $data and $this->data
$data->page_title = 'example';

This also gives you the possible side benefit of being able to type hint against an object in method signatures as you are passing this data around or injecting it as dependency.