Returning instance variable V/s returning direct object

Question

I am looking into source code of the website which was built in the year 2009, it was a custom framework.

What is the difference?

<?php
class DbAccess {

    private static $instance;

    /**
     * Returns the instance of the DB Class
     */

    public static function getInstance()
    {
        self::$instance = new DbAccess();
        return self::$instance;
    }
}

V/s

<?php
class DbAccess {


    /**
     * Returns the instance of the DB Class
     */

    public static function getInstance()
    {
        return new DbAccess();
    }

}

I have worked on couple of custom-made frameworks and set of libraries with different patterns but, at times, I saw the method of returning instance was via self::$instance and at times, it was directly returning via new

Which is a good practice? considering upcoming versions of PHP.

The first is an attempt at a [Singleton](http://en.wikipedia.org/wiki/Singleton_pattern). This doesn't quite get it right though as it's constantly creating new instances. The second is essentially a crappy factory method. Neither is preferable. See http://en.wikipedia.org/wiki/Singleton_pattern#Drawbacks — Phil, Mar 05 '13 at 06:09
Yep, totally get that ! But, why would many people follow the second pattern? — Karma, Mar 05 '13 at 06:11

score 5 · Accepted Answer · edited May 23 '17 at 11:49

5

Both are not good practice when talking about OOP. The first method as others already pointed out looks like somebody trying to implement the singleton pattern and failed. What is so bad about a singleton in terms of OOP. It introduces tight coupling, hidden dependencies and global state. Basically it is just a fancy way of using the global keyword.

Now for your second example. It is basically the same thing with the exact same drawbacks.

Note both examples create a new database connection every time the method is called. Which is... not optimal.

Now what I would have done is something like the following (using dependency injection):

Lets say you have some class which needs database access you would do something like the following:

class Foo
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

    public function methodWhichNeedsDatabaseAccess()
    {
        // do something with $this->db
    }

}

// or whatever instance for database access you are using
$db = new PDO(dsn);
$foo = new Foo($db);
$foo->methodWhichNeedsDatabaseAccess();

edited May 23 '17 at 11:49

Community

1
1

answered Mar 05 '13 at 11:23

PeeHaa

66,697
53
182
254

Understood. But then, what is the point of having `return self::$instance;` ? I mean, it was a practice earlier, why is it carried on even today (though, at a minimized rate)? – Karma Mar 05 '13 at 11:47
Because some people don't have a very good understanding of proper OOP and testability. – PeeHaa Mar 05 '13 at 11:50
-__- that was useful! So, from the answer can I conclude that using getInstance() method which returns the singleton instance may not be a great practice of returning an object? but, initializing an object is! ? – Karma Mar 05 '13 at 11:55
1

The point is two fold. 1. don't create an instance of a class inside another class when possible because it introduces tight coupling. 2. don't use `public static` methods. Because when you do you are not doing OOP (i.e. working with objects), but are namespacing functions. – PeeHaa Mar 05 '13 at 11:59

score 0 · Answer 2 · answered Mar 05 '13 at 06:10

The static functions are both returning a new instance of the object. The first example allows you to reference that instance statically again in the future, although it's stored in a private static, so it would only be accessible from members (functions) within that class.

score 0 · Answer 3 · answered Mar 05 '13 at 06:11

The first example looks like an example of a singleton, however, it isn't wrapped in the typical if exists wrapper. The singleton is good practice when you only want one instance of the class to ever be created:

public static function getInstance() {
  if (!isset(self::$instance))
    self::$instance = new DbAccess();
  return self::$instance;
}

The way this works is that you can only access this class through that one instance (so that you're not opening multiple connections to the same database).

They may follow the second pattern if they want to control the constructor. You can make the constructor private (so that only that class can call the constructor and create new objects). This is common in factories.

"The singleton is good practice when you only want one instance of the class to ever be created". No it is not. It is simply the `global` keyword only written differently. A singleton in PHP and OOP has no place. — PeeHaa, Mar 05 '13 at 11:05

Returning instance variable V/s returning direct object

3 Answers3