Jump to content

OOP Failure


ShibSta
 Share

Recommended Posts

I'm not sure what I am doing wrong but I am attempting to create a hook system that I can use with modules... Could someone please take a look at the code and point me in the right direction? I've never really dealt with OOP so please be detailed in your response.

 

There are no errors, it's just not working as expected.

 

<?php
new System();
class System {
public function __construct() { 
	$this->admin = new Admin( &$this );
}

public function add_hook( $tag, $method, $class ) {
	$this->hooks_added[$tag][] = array( 'method' => $method, 'class' => $class );
}

public function do_hook( $tag, $msg ) {
	if( !isset( $this->hooks_registered[$tag] ) )
		$this->hooks_registered[$tag] = $msg;
	else
		$this->fatal_error( "1002." . __LINE__, "Duplicate hook tags defined." );
	if( isset( $this->hooks_added[$tag] ) && is_array( $this->hooks_added[$tag] ) ) {
		foreach( $this->hooks_added[$tag] as $hook ) {
			$hook['class']->$hook['method']();
		}
	}
}

public function fatal_error( $num = "Undefined", $title = "Undefined", $msg = "Undefined" ) {
	$html = 'Error ' . $num . ': ' . $title;
	echo $html;
}
}

class Admin {
public $_system;
public $admin_menu = array( 
	0 => array( 
		"name" => "Link 1", 
		"params" => "a=link1"
	),
	5 => array( 
		"name" => "Link 2", 
		"params" => "a=link2"
	)
);

public function __construct( $_system ) {
	$this->_system = $_system;
	$this->dashboard = new Dashboard( &$_system );
	$this->build_menu();
}

private function build_menu() {
	$this->_system->do_hook( "admin_menu", "Modify the administrative menu." );
	ksort( $this->admin_menu );
	$html = '<ul>';
	foreach( $this->admin_menu as $link ) {
		$html .= '<li>' . $link['name'] . '</li>';
	}
	$html .= '</ul>';
	echo $html;
}

public function add_menu_item( $name, $params, $sort ) {
	if( isset( $this->admin_menu[$sort] ) ) {
		$this->add_menu_item( $name, $params, (int) $sort + 1 );
	} else {
		$this->admin_menu[$sort] = array( "name" => $name, "params" => $params );
	}
}
}

class Dashboard extends Admin {
public $_system;
public function __construct( $_system ) {
	$this->_system = $_system;
	$this->_system->add_hook( "admin_menu", "custom", &$this );
}

public function custom() {
	$this->add_menu_item( "Dashboard", "a=dashboard", 0 );
}
}
?>

 

Thanks

 

Link to comment
Share on other sites

There are some declarations missing, a property needs declaring before you can assign to it/give it state. Though as it is 'before coffee time' I may have misread that, but personally I would have error_reporting(E_ALL|E_STRICT); running on this little project... You never know.

 

Valid question from previous poster though; What's the actual issue here?

 

Rw

Link to comment
Share on other sites

The only thing that sticks out to me is the first line.  You create a new instance of System without assigning it to anything.  That particular instance is therefore inaccessible.

 

There are some declarations missing, a property needs declaring before you can assign to it/give it state.

 

Not really. In PHP you can create public properties on-the-fly by assigning a value to them.

 

That's a feature I always hated.  I like my objects to have concrete interfaces.

Link to comment
Share on other sites

Sorry, I had not slept in 32 hours at the time of posting this - I kinda forgot to explain the actual problem. lol

 

Anyway, the problem is that the "Dashboard" menu item is not added to the menu from the hook. I think it's referencing things incorrectly but I cannot find out where. The code is standalone, if you copy it into a php file and run it - you will see what it's currently doing.

 

Currently, it outputs:

  • Link 1
  • Link 2

 

However, it should output:

  • Link 1
  • Dashboard
  • Link 2

 

Thanks again. :)

Link to comment
Share on other sites

That's a feature I always hated.  I like my objects to have concrete interfaces.

Well said that man, in some circles, this is considered to be good practise of code creation.

However, it should output:

 

    * Link 1

    * Dashboard

    * Link 2

 

Someone will no doubt beat me to it, but I am intrigued now, so I shall have a play and post later on tonight.

 

Rw

Link to comment
Share on other sites

Right, gone through with a nit comb and found an extra } that wasn't needed, and on the first if/else clause there were no braces, though I am not sure if this would 'break' it.

 

Formatted a bit better too now - have a try at that - I haven't altered the structure, but I would approach this a little differently. Hey ho.

 

EDIT: I give up, the forum software is munging the tabs up, in my editor this is formatted ok.

 

Rw

 

<?php
new System();
class System {

public function __construct(){ 
        $this->admin = new Admin( &$this );
}

public function add_hook( $tag, $method, $class ) {
       $this->hooks_added[$tag][] = array( 'method' => $method, 'class' => $class );
}

public function do_hook( $tag, $msg ) {
if( !isset( $this->hooks_registered[$tag])){
	$this->hooks_registered[$tag] = $msg;
}
else{
	$this->fatal_error( "1002." . __LINE__, "Duplicate hook tags defined." );

		if( isset( $this->hooks_added[$tag] ) && is_array( $this->hooks_added[$tag])){
			foreach( $this->hooks_added[$tag] as $hook ){
				$hook['class']->$hook['method']();
			}
		}

}


public function fatal_error( $num = "Undefined", $title = "Undefined", $msg = "Undefined"){
$html = 'Error ' . $num . ': ' . $title;
echo $html;

}

}

class Admin {

public $_system;
public $admin_menu = array(0 =>
				    array("name" => "Link 1", "params" => "a=link1"),5 =>
				    array("name" => "Link 2", "params" => "a=link2"));


public function __construct($_system){
$this->_system = $_system;
$this->dashboard = new Dashboard( &$_system );
$this->build_menu();
}

private function build_menu(){
$this->_system->do_hook( "admin_menu", "Modify the administrative menu." );
ksort( $this->admin_menu );

$html = '<ul>';
foreach( $this->admin_menu as $link ){
	$html .= '<li>' . $link['name'] . '</li>';
}
	$html .= '</ul>';
	echo $html;
}


public function add_menu_item( $name, $params, $sort ){
if( isset( $this->admin_menu[$sort])){
	$this->add_menu_item( $name, $params, (int) $sort + 1 );
}
else{
	$this->admin_menu[$sort] = array( "name" => $name, "params" => $params );
}
}

class Dashboard extends Admin {
public $_system;

public function __construct($_system){
	$this->_system = $_system;
	$this->_system->add_hook( "admin_menu", "custom", &$this );
}


public function custom(){
$this->add_menu_item( "Dashboard", "a=dashboard", 0 );
}

}
?>

Link to comment
Share on other sites

The only thing that sticks out to me is the first line.  You create a new instance of System without assigning it to anything.  That particular instance is therefore inaccessible.

 

There are some declarations missing, a property needs declaring before you can assign to it/give it state.

 

Not really. In PHP you can create public properties on-the-fly by assigning a value to them.

 

That's a feature I always hated.  I like my objects to have concrete interfaces.

 

Nothing the below can't fix ;)

 

function __set($property, $value) {
    throw new Exception("Property $property does not exist.");
}

Link to comment
Share on other sites

@rwwd: I wasn't receiving an error so I'm not sure how there could have been an extra brace, it would have given a fatal error... Anyway, I'm on my mobile phone right now - I'll look at the changes you've made to the code hen I get home.

 

I am completely new to writing OOP and dealing with the relationship between objects, therefore, I am open to suggestions and would love to know how you would approach it. I don't know anything about design patterns or the common practices in dealing with the ability to access one object from within another so I'd appreciate and additional tips. :)

 

Thanks again

Link to comment
Share on other sites

@shibsta:  Well there is no time like the present to learn OOP and the HUGE benefits you will get from utilising it, just be wary about the versions of php that you use.  PHP5 has has the class engine rewritten now, so things like 'var', 'public', 'private', 'protected', and I think 'static', are all php5, and wouldn't work in php4 same goes for some of the functions.

 

I suggest that your first port of call is to see how the relationship between functions(methods) an objects, and calling one from the other before you start on other things, one thing that I find particularly useful is the __construct() method, I use it a lot now, but I guess that's a lesson for another day.

 

Best of luck squire.

 

Rw

Link to comment
Share on other sites

@rwwd, I appreciate the time you took to assist me, but you actually removed braces that were required; the code now has fatal errors. Also, you changed some of the logic that didn't need changed. On the first if/else, the else was meant to be only 1 line and not a block. I know OOP is huge and I need to learn it or I'll have quite some trouble around this time next year when I'm a computer science major. I am trying to learn the basics now and am stuck with the problem I've posted in this topic. This isn't for a website or part of a pre-existing script, I am doing this as an educational experiment, on my own time, to try and understand OOP a little better. Also, I'm running PHP 5.2.9

 

The problem remains and I'd appreciate it if someone could look into how I'm actually calling methods from other objects and how I'm passing objects through references; I believe that is where my problem lies. However, I do not know enough about it to resolve the issue on my own.

 

Thanks again,

Link to comment
Share on other sites

This thread is more than a year old.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.