Is this bad Object Oriented PHP?

Question

I've created a Display object that contains the header, sidebar and footer:

class Display { protected $framework; public $mysql; public function __construct() { $this->mysql = new MySQL(); $this->framework .= $this->header(); $this->framework .= $this->body(); $this->framework .= $this->sidebar(); $this->framework .= $this->footer(); } private function header (){ /* blah */ } private function body (){ } private function sidebar (){ /* blah */ } private function footer (){ /* blah */ } public function displayPage(){ print $this->framework; } }

On each page I've created a object that extends the Display object, with the code for the body:

class IndexPHP extends Display { public function body(){ $this->user = new User(); return '<div class="body">Hello ' . $this->user->getName() . '</div>'; } } $page = new IndexPHP(); $page->displayPage();

Have I created a problem by nesting the objects too much? For example, in the User object, how do I access the already initiated MySQL object?

class User { protected $name; public function __construct() { $this->id = /* MySQL object query here */ } }

Alex Weinstein · Accepted Answer · 2009-07-29 20:14:30Z

The issue with the approach you've given is that you're not following any sort of "separation of powers" principles. Your Display object has a database in it, along with the logic as to how to connect to it; that's probably not the best approach ("the God object"). It may be a better idea to follow MVC (model-view-controller) principles, where you have one class that knows something about your model (database), another that knows how to transform the model into objects that will be presented (controller), and the third that actually shows the data with all of its CSS goodness (view, frequently just a PHP template file).

I'd recommend you take a look at an existing MVC framework - I use QCubed (http://qcu.be), there are others - Symfony, Zend, CakePHP. They all offer you a great way to separate your code cleanly, which ultimately results in maintainability.

I don't think it's that bad. He just needs to pass the dependencies in from outside.
It's unmaintainable. It would work OK for a small toy project, but I wouldn't ship code like this for a production system.
Thanks for taking the time to comment. I'm new to OOPHP, so the guidance really helps.
Whether it's maintainable really depends on how much of an application he piles on top of it. It isn't wise to choose a design without considering the context. Using Symfony for a 5 page portfolio site and a contact form, is a bad call in my opinion.

troelskn · Accepted Answer · 2009-07-29 20:14:48Z

For example, in the User object, how do I access the already initiated MySQL object?

You pass it in the constructor:

class User { protected $name; public function __construct($mysql) { $this->id = $mysql->something(); } }

Right....I think I'll try and flatten things a bit....I guess nesting and objects don't mix well. Thanks

rojoca · Accepted Answer · 2009-07-29 21:02:48Z

If you override functions in child classes (for example your IndexPHP class overrides body()) you should make them protected instead of private.

Also, it's good practice to only do simple things, like assign values, in the constructor. Your Display constructor does all the work in the class it might be better to move the construction work to displayPage:

public function __construct(MySQL $mysql) { $this->mysql = $mysql; } public function displayPage($rebuild=false) { if(empty($this->framework) || $rebuild) { $this->framework = $this->header(); $this->framework .= $this->body(); $this->framework .= $this->sidebar(); $this->framework .= $this->footer(); } print $this->framework; }

Collectives™ on Stack Overflow

Is this bad Object Oriented PHP?

3 Answers 3

4 Comments

1 Comment

Comments

Linked

Hot Network Questions