views:

119

answers:

2

I have the following PHP code;

<?php

component_customer_init();
component_customer_go();


function component_customer_init()
{
    $customer = Customer::getInstance();
    $customer->set(1);
}
function component_customer_go()
{
    $customer = Customer::getInstance();
    $customer->get();
}

class Customer
{
    public $id;
    static $class = false;
    static function getInstance()
    {
        if(self::$class == false)
        {
                self::$class = new Customer;
        }
        else
        {
                return self::$class;
        }
    }


    public function set($id)
    {
        $this->id = $id;
    }

    public function get()
    {
        print $this->id;
    }

}
?>

I get the following error;

Fatal error: Call to a member function set() on a non-object in ....../classes/customer.php on line 9

Can anyone tell me why I get this error? I know this code might look strange, but it's based on a component system that I'm writing for a CMS. The aim is to be able to replace HTML tags in the template e.g.;

<!-- component:customer-login -->

with;

<?php component_customer_login(); ?>

I also need to call pre-render methods of the "Customer" class to validate forms before output is made etc.

If anyone can think of a better way, please let me know but in the first instance, I'd like to know why I get the "Fatal error" mentioned above.

Cheers

+2  A: 

Well, I think your Customer::getInstance() method is flawed. It should look like this:

...
static function getInstance()
{
    if(self::$class == false)
    {
            self::$class = new Customer;
            return self::$class; // ADDED!!
    }
    else
    {
            return self::$class;
    }
}
....

In the if(self::$class == false) branch you are creating the instance of the class, but you dont return it.

You could also rewrite it as such:

static function getInstance()
{
    if(self::$class == false)
    {
        self::$class = new Customer; 
    }

    return self::$class;
}

To make it a bit shorter.

Max
+1 Your edit improved your answer.
John Conde
Cheers Max, you saved me a headache. It's amazing what you miss when you're tired.
Webbo
+1  A: 

DRY: Don't Repeat Yourself

static function getInstance()
{
    if(self::$class == false)
    {
        self::$class = new Customer;
    }
    return self::$class;
}

And for Sinlgetons it is also important to prevent __clone() from being used. Making it private should solve that problem:

private function __clone() {}
John Conde