Critique My Codeigniter Custom CMS Pages Model

I am currently developing a custom CMS being built on top of Codeigniter and was wondering if you can spot any flaws in my page fetching model code. The page fetching model is not entirely complete but the main functionality for retrieving a page is done, as well as retrieving modules assigned to a page (a module is really just a widget).

Can this model be better in some parts, perhaps in relation to the joins I am doing? although not really joins, but multiple queries to pull out bits of related info the pages like modules and media.

```
db;

$query =
$db
->where('page_status', 1)
->where('page_slug', strtolower($page_slug))
->get('pages')
->result_array();

$page_id = $query[0]['id'];

$query['modules'] =
$db
->select('modules.module_name, modules.module_slug, modules.id moduleid')
->where('page_id', $page_id)
->join('pages_modules lpm', 'moduleid = lpm.module_id')
->order_by('module_order', 'asc')
->get('modules')
->result_array();

/*$query['media'] =
$db
->select('lucifer_media.media_file_name, lucifer_media.media_file_extension, lucifer_media.media_directory')
->join('lucifer_pages_media', 'lucifer_pages_media.page_id = '.$page_id.'')
->get('lucifer_media')
->result_array();*/

if ($query) {

return $query;

} else {
return false;
}
}

public function fetch_navigation()
{
$result = $this->db->order_by("nav_order", "asc")->where('published', 1)->get('navigation')->result_array();
return $result;
}

public function fetch_layout($id)
{

$result = $this->db->where('id', $id)->get('layouts')->result_array();

return $result[0];

}

}

?>

I don't know Codeigniter so i can't comment on "is this a propper model". For my answer i'm just going to assume it is.

It's not much code so i'm going to focus on some details:

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

Those 5 lines do absolutely nothing. I'd remove them. If you need a constructor later, add it later.

public function fetch_navigation()
  {
    $result = $this->db->order_by("nav_order", "asc")->where('published', 1)->get('navigation')->result_array();
    return $result;
  }

This is a pretty hard to read (113 Chars in that line). I'd go for something like this (to say with your formatting from above)

public function fetch_navigation()
  {
      return $this->db
          ->order_by("nav_order", "asc")
          ->where('published', 1)
          ->get('navigation')
          ->result_array();
  }

$db = $this->db;

Seems unnecessary to create a local variable, those 6 extra chars shoudn't hurt. I needed to look twice to see where that variable is from and if its local because changes are made to it.

So far those are really minor notes.

Unreachable code ?

if ($query) {
  return $query;
} else {
  return false;
}

you are setting $query['modules'] = ... so even if that is null $query will at least contain

array("modules" => null);

and that will always be true.

StackExchange Code Review Q#48, answer score: 4