PHP OOP Good practice for accessing methods?

Question

I have some code that often looks like this:

private $user;

public function __construct()
{
    $this->user = User::getInstance(); //singleton
}

public function methodOne()
{
    return $this->user->foo();
}

public function methodTwo()
{
    return $this->user->foo2();
}

public function methodThree()
{
    return $this->user->foo3();
}

I figure if I set user property to the instance I can reuse a shorter name in my methods (well in this case it's not that much shorter). I also thought doing it this way might save a little resources (beginning to doubt it), but when I look at other people's code I rarely see people do this. They would usually just call:

User::getInstance()->foo();
User::getInstance()->foo2();
User::getInstance()->foo3();

Is there any sort of best practice for this? Maybe if it's not a singleton class you might do it this way? Or maybe you should never do it this way? Hope to get some clarification, thanks.

Edit: Incase there is any misunderstanding I'm just wondering if I should the first example with creating a property to store the instance vs this:

public function methodOne()
{
    return User::getInstance()->foo();
}

public function methodTwo()
{
    return User::getInstance()->foo2();
}

public function methodThree()
{
    return User::getInstance()->foo3();
}

Actually now that I think about it this may be less code as I don't need the constructor...

Answer 1

There are indeed some problems with your approach.

• It is not clear that your class depends on the User class. You can solve this with adding User as a constructor parameter.
• Singletons are often bad practice. Your code demonstrates why: it is globally accessible and hence difficult to track dependencies using it (this points to the above problem).
• Static methods are too often used as global access points (in response to what you see people usually do User::method()). Global access points give the same problem as singletons. They are also a tad more difficult to test.

I also don't see the point in repeating the User object with your new object, unless you would use eg the adapter pattern. Maybe if you could clarify this I would be able to come up with a better alternative than the generic:

class Foo {
    public function __construct(User $user) {
        $this->user = $user;
    }
    public function doXsimplified() {
        $this->user->doXbutMoreComplex($arg1,$arg2, $arg20);
    }
}