CodeIgniter Forums

Full Version: Refactoring controllers. :/
You're currently viewing a stripped down version of our content. View the full version with proper formatting.

El Forum

[eluser]pluus[/eluser]
Code:
<?php
class News extends CI_Controller {

public function __construct()
{
  parent::__construct();
  $this->load->model('news_model');
}

public function index()
{
  $data['news'] = $this->news_model->get_news();
  $data['title'] = 'News archive';

$this->load->view('templates/header', $data);
$this->load->view('news/index', $data);
$this->load->view('templates/footer');
}

public function view($slug)
{
  $data['news_item'] = $this->news_model->get_news($slug);

  if (empty($data['news_item']))
  {
   show_404();
  }

  $data['title'] = $data['news_item']['title'];

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

public function create()
{
  $this->load->helper('form');
  $this->load->library('form_validation');
  $data['title'] = 'Create a news item';
  $this->form_validation->set_rules('title', 'Title', 'required');
  $this->form_validation->set_rules('text','text','required');

  if($this->form_validation->run() === FALSE)
  {
   $this->load->view('templates/header', $data);
   $this->load->view('news/create');
   $this->load->view('templates/footer');
  }
  else
  {
   $this->news_model->set_news();
   $this->load->view('news/success');
  }
}
}?>

I've been going through the codeigniter tutorials and the final code ended up like that.
However, several methods in this controller are calling their own template files(header/footer) here and there. Since I'm a newbie, I'm not sure if it's a good idea to allow each method to call their own templates - they're kinda duplicating! Is this the correct way of programming controllers to let each method do that?? Is there a way to refactor it so that I can minimize calling the templates?

El Forum

[eluser]TWP Marketing[/eluser]
pluus
I've edited your controller to add a generic display method at the bottom, The other methods setup the data array and call the display method. I also added return; to some of them. The concept is called DRY - Don't Repeat Yourself

[quote author="pluus" date="1344356577"]
Code:
<?php
class News extends CI_Controller {

public function __construct()
{
  parent::__construct();
  $this->load->model('news_model');
}

public function index()
{
  $data['news'] = $this->news_model->get_news();
  $data['title'] = 'News archive';
  $data['view_path'] = 'news/index'; // this is the view to be shown as content
  $this->show_page($data); // this will display your page
  return;
}

public function view($slug)
{
  $data['news_item'] = $this->news_model->get_news($slug);

  if (empty($data['news_item']))
  {
   show_404();
  }

  $data['title'] = $data['news_item']['title'];
  $data['view_path'] = 'news/view'; // this is the view to be shown as content
  $this->show_page($data); // this will display your page
  return;
}

public function create()
{
  $this->load->helper('form');
  $this->load->library('form_validation');
  $data['title'] = 'Create a news item';
  $this->form_validation->set_rules('title', 'Title', 'required');
  $this->form_validation->set_rules('text','text','required');

  if($this->form_validation->run() === FALSE)
  {
   $data['view_path'] = 'news/create'; // this is the view to be shown as content
  }
  else
  {
   $this->news_model->set_news();
   $data['view_path'] = 'news/success'; // this is the view to be shown as content
  }
  $this->show_page($data); // this will display your page
}

public function show_page($data)
{
  $this->load->view('templates/header', $data);
  $this->load->view($data['view_path'], $data);
  $this->load->view('templates/footer');
  return;
}

}?>

...[/quote]

El Forum

[eluser]pluus[/eluser]
That's much better Big Grin Thank you!