Problem
Recently I started working with TinyMVC, wrote a simple menu model and I have few questions for those who are using it… or used before.
- For the following code should I keep it as Model or as a Plugin?
- How should I implement it in the view and use it on every page that is required without breaking the ideea of MVC and without rewriting again and again for each controller?
- Any improvements to the code?
- Need the MySQL tables?
<?php
class Menu_Model extends TinyMVC_Model
{
public function __construct()
{
parent::__construct();
}
public function listMenu()
{
return $this->db->query_all("SELECT * FROM menu_links WHERE is_deleted = 0 ORDER BY position");
}
public function listCategorys($menuLinkId)
{
return $this->db->query_all("SELECT * FROM menu_subcategorys WHERE menuLinkId = ? AND is_deleted = 0 ORDER BY position", array($menuLinkId));
}
public function buildMenu()
{
$this->listMenu = $this->listMenu();
foreach($this->listMenu as $this->listMenuKey => $this->listMenuValue)
{
$this->listCategorys = $this->listCategorys($this->listMenuValue['menuLinkId']);
if(!empty($this->listCategorys))
$this->listMenu[$this->listMenuKey]['child'] = $this->listCategorys;
}
return $this->listMenu;
}
}
Solution
Sorry you had to wait so long, answers here tend to take some time and I’ve been on vacation the last week, so I wasn’t keeping up with this site the way I usually do. So, here we go…
Q: “Any improvements to the code?”
First off, I don’t think you quite understand inheritance. If you are extending one class from another it automagically inherits all properties and methods of the parent class. Therefore, there is no need for you to recreate a __construct()
method if all you are going to do with it is call the parent one. The only reason to do it the way you have is if you had called additional code in the new constructor before or after calling the parent one. Example:
public function __construct() {
$changedParentVar = 'new value';
parent::__construct();
echo 'we changed the "changedParentVar" inside the parent constructor method before running it';
}
Clarification: I’ll be honest, I’m not quite sure if this stands true for deep inheritance. Say I have three classes, each inherits from the previous, and only the first has a constructor. I don’t know if the third will still have the original constructor. I know it will still have all the other methods, but for some reason I want to say that magical methods are different, yet I can’t find anything at the moment to prove this one way or another. So this statement is here to cover my ass 🙂
I’d find some way of combining your “list” methods, something like…
public function list($from, $where, $and = '') {
if( ! empty($and)) { $and = "AND $and"; }
return $this->db_query_all("
SELECT *
FROM $from
WHERE $where
$and
ORDER BY position
");
}
If you are not going to be reusing a variable ($this->listMenu
or $this->listCategorys
or a foreach variable) there is no need to make them properties. The only reason you should make a variable a property is so that it can be used outside the current method. That being said, take a look at that foreach loop… There is no need to set the key/value pair as a class property as they will not be reused before they are reset, and I doubt you will reuse just the last one. This also stands true for those other two properties I mentioned, they should be variables, not properties. So the buildMenu()
method should be rewritten like so.
public function buildMenu() {
$listMenu = $this->listMenu(); //Keep this line how you have it if you are using listMenu in another method you did not include here...
foreach($listMenu as $key => $value) {
$listCategorys = $this->listCategorys($value['menuLinkId']);//BTW its categories not categorys... been bothering me...
if( ! empty($listCategorys)) { $listMenu[$key]['child'] = $listCategorys; }
}
return $listMenu;
}
By the way, please, for everyone’s sanity, always use braces {}
around any statements, even if they are only one line. Its so much easier to read.
Q: How should I implement it in the view and use it on every page that is required without breaking the ideea of MVC and without rewriting again and again for each controller?
You should not implement it in the view, only in the controller. Views should only contain minimal PHP that helps output data.
I’m not sure what you mean about having to rewrite it, please clarify. In the mean time here are a few examples of how I would implement it.
If my controller is a class:
class Controller extends Menu_Model { }
//OR
class Controller {
public function __construct() {
include 'path/to/Menu_Model.php';
$this->Menu_Model = new Menu_Model();
}
}
If my controller is just a list of PHP functions or procedural:
include 'path/to/Menu_Model.php';
$Menu_Model = new Menu_Model();
Since you are using MVC, I would also suggest that you take a look at autoloading.
Q: For the following code should I keep it as Model or as a Plugin?
Not sure what you mean by plugin. Model should be fine though.