Class instances inside other class instances

[bunner_bob]

I'm trying to put several class instances (header, body, sidebar) inside another class instance (page). I figured out a way to do it but I'm not sure it's the best way - perhaps someone here can advise me.

All of my classes require at least two "calls" (if that's the right term). One to instantiate them (and in some cases, via constructors, set up initial parameters), a second to execute them and compile their results into an html template.

For instance:

class Body {

  var $formatted_body;

  function Body() {
    // set initial parameters
  }

  function GenerateBody() {
    // run a number of other functions to come up with 
    // the remaining parameters for the page
   // read in body_template.html
    // swap in parameters for placeholders in the template
    $this->formatted_body = // template with parameters set
  }

}

I have another class that takes all the formatted sections, combines them with an HTML template and outputs that to the browser

class Page {
  var $head;
  var $body;
  var $sidebar;

  function GeneratePage() {
    // create array of the three page sections
    // open the html template for the page
    // go thru the template and swap each array element in for its placeholder
    // echo the populated template to the browser
  }

}

Calling the whole thing, I say:

$page =& new Page();

$head =& new Head();
$head->GenerateHead();
$page->head =& $head->formatted_head;

$body =& new Body();
$body->GenerateBody();
$page->body =& $body->formatted_body;

$sidebar =& new Sidebar();
$sidebar->GenerateSidebar();
$page->sidebar =& $body->formatted_sidebar;

$page->GeneratePage();

So basically I appear to be completely running through each page component class, getting the results from it, sticking it into another class and then outputting.

I'm wondering if I shouldn't just be creating the class instances of the page components within the overall Page class instance, and then "running" their "Generate" functions inside the Page instance - if I can do that, or if it even makes sense. I think it would look something like this:

$page =& new Page();

$page->head =& new Head();
$page->body =& new Body();
$page->sidebar =& new Sidebar();

// this is the part I'm not sure about...

$page->head->GenerateHead(); // run GenerateHead() on the instance of the "Head" class INSIDE the "Page" class
$page->body->GenerateBody(); // ditto for Body
$page->sidebar->GenerateSidebar(); // ditto for Sidebar

$page->GeneratePage();

The question - is this smart? Sane? Possible? Is this the right syntax for accessing an instance-within-an-instance?

And p.s. - am I using my pass-by-references right? ("=&") It's my first time out with these things...

Thanks for any advice you can offer.

• Bob

[Shrike]

Your second example makes the most sense. Depending on what the section classes and their methods do it may make sense to simplfy the object model a little. If all you want to do is include HTML fragments, then you could create a factory object which did the object creation, and pass it a parameter so it would know which type of object to create (head, nav, bottom or whatever)

<?php

class Page {

  var $newobj;
  var $element;
  var $elements;
  var $allowed = array ( "head", "footer" );

  function page(){
    // Init Page class
  }

  function &factory(&$obj){
  	if(in_array($this->element, $this->allowed)){
    	return new $obj();
    }else{
    	die("Page factory doesn't create type $this->element");
    }
  }

  function addElement($element){
    $this->element =& $element;
    $this->elements[] =& $this->factory($this->element);
  }

  function render(){
    foreach($this->elements as $value){
      echo $value."<br>";
    }
  }
}

class head {}
class footer  {}

$page = new Page();

$page->addElement("head");
$page->addElement("footer");
$page->addElement("invalid");

$page->render();

?>

EDIT: refined it a bit 🙂

The factory method allows you to control what objects can be created. Sorry if I messed up the syntax I've not had enough coffee this morning 🙂

[Oersoep]

I assume a head is useless if it's not inside a page and a page has only one head. Therefor you could make the head a little more 'private'.
Let the head mind it's own business, let the page mind his head so all you have to mind is the page.

You can instatiate the body, sidebar and head in the constructor of the page. That way your main script doesn't need to do things with the minions of the page instance, but let the page handle it.

class page{

   var $head;
    // etcetera

   function page(){

  $this->head = new head();
  $this->head->generate();

  // etcetera

   }

function render(){

  // show things

}

}

// main script:

$page = new page();
$page->render();

[bunner_bob]

Oersoep - I read somewhere it was a bad idea to have constructors doing actual work - safer to let them set up variables & such and do the work in a separate function call. I'm not sure WHY this would be safer, what the risk would be otherwise. Maybe someone here knows.

Shrike - I like your idea. I feel like I'm headed in the other direction though. My Body class for the admin portion of my site is already about 2600 lines - it has sections for displaying tables of items, editing items, errorchecking submissions, submitting, deleting, and generating different size images for thumbnails and insets within articles. A lot of these sections occur multiple times (errorcheck for dates, for content, for images, etc.) - though I've been able to consolidate some of them. But I'm thinking it might be more tidy to separate it into table display class, edit content class, submit class - something like that. Not sure if it makes a difference though. I suppose if I'm not including the table and submit classes when I'm editing, that's fewer lines for php to read through (even though it isn't executing anything in them). Or maybe it's no difference. What do you think?

• Bob

[Shrike]

If a class has got to be 2600 lines something has gone a bit awry. One of the key ideas behind OO code is to write modular, portable and abstracted classes that can have lots of uses. I'm not preaching here because I do the same thing as you, but from reading various articles and my limited experience of OO code it seems that keeping your classes simple is a good standard to aim for.

As you mention you are thinking of splitting the various functions up, and this is a good idea. Unfortunately PHP4 does not really encourage you to do this. I've played a bit with PHP5 and it really encourages you to get into the OO mindset, with it's new abstract classes and interfaces etc etc. I've also been reading a fair bit of stuff on http://www.phppatterns.com and that's got me to thinking alot. A particular pattern, named the Observer pattern, is a powerful tool for maintaining a controlled object model. Worth a read 🙂

One other thing, as Oersopp mentions, the methods which would only be used internally by a class should be defined that way. Again PHP4 has no good way of doing this, PHP5 has public, private and protected properties to achieve this. Your right that the constructor should not do any processing at all, it should merely instantiate the object's needed properties.

[bunner_bob]

Shrike - I've got it almost figured out now. Only problem is when I run my "render" function what gets echoed is "Object".

I can say:
echo $page->head->FormatHead();
and it echoes it just fine.

But I don't want to echo it - I want to format it and "store" the formatted head in $page->head, so when I render (echo) it (or in my case, swap it into the template for the overall page) it's already formatted.

I suppose I could set up another variable in the Page class called "formatted_head" or something. Then say:
$page->formatted_head = $page->head->FormatHead();

and then when I render the page, I'd render $formatted_head instead of $head.

However, seems there should be a way to do it the first way, and get the right stuff out instead of seeing "Object".

Does this make sense? I'll give you code if you need it.

[Shrike]

If $page->head->formatHead() works then just call that at the correct time, rather than storing in a variable. It'd be good to see some code though 🙂