2

Just a question on standards.

I've created a wrapper class for PHP session management, that helps automatically organize session data based on certain internal modules accessing it. It's designed as a singleton, using a getInstance() method to instantiate, since there will only be a single session at a given time. Also, this came as a benefit to me, as I am able to prevent instantiation of the session object in the (albeit probably limited) chance that session_start() fails. As for an example:

    public static function getInstance(){
        if(!self::$_instance || !session_id()){
            if(session_start()){
                self::$_instance = new self(session_id());
            }else{
                return;
            }

        }
        return self::$_instance;
    }

My question is; although the use of a gateway getInstance() method works naturally here for a few reasons, is it common/good practice to implement public static getInstance() or create() methods in classes to control object creation if the object is reliant on external conditions?

I just find myself sticking to a convention of providing getInstance() in the case of singletons, and create() in the case of multiple instance objects.

TL;DR: I keep using getInstance() and create() methods to control all object instantiation. Am I doing it wrong?


EDIT: Refining my question a bit; Aside from using getInstance() for singletons, is my constructor wrapping with create() methods serving less purpose and more leaning towards bad convention? Should I be throwing Exceptions from the true constructor, or continue returning false from a create()?

Dan Lugg
  • 20,192
  • 19
  • 110
  • 174

3 Answers3

2

Singletons are generally considered "bad"; see this section here for a flame war on the topic.

That said, using factory methods or factory classes to create objects is generally considered good, so you're fine there :)

I personally use the symfony dependency injection component (can be installed in any project without using the symfony framework) to simplify dependency injection and avoid singletons where it seems appropriate.

I do still use some singletons, where it makes sense to me; loggers and factory objects, for example, seem to naturally be single to me, so I make them that way. The idea is that global functionality (e.g. a factory) is fine, but global state is bad, I believe.

With regards to your amended question on whether to throw an Exception or whether to return false from your create() call; it depends on whether your application can continue successfully without the created object if not. If, for example, you were creating a database connection that is necessary to create a page, then throw an Exception. If you're doing something less essential, return false and continue on your merry way :)

Community
  • 1
  • 1
El Yobo
  • 14,823
  • 5
  • 60
  • 78
  • 2
    Singleton pattern ain't bad, that's for sure. Overusing it is bad (as overusing anything is bad). :) – egis Nov 30 '10 at 06:25
  • All things in moderation, I suppose :) – El Yobo Nov 30 '10 at 06:28
  • Thanks **El Yobo**; I'll have a look at your suggestion :) – Dan Lugg Nov 30 '10 at 06:50
  • Just a further note; I suppose globalizing an object registry by designing the registry as a singleton would be considered just as "bad" (as per those in the anti-singleton camp) as designing the objects intended for use with the registry as singletons to begin with. – Dan Lugg Dec 02 '10 at 22:51
  • Maybe... but that's what I've done. I suppose that having one singleton is better than having many, however :) – El Yobo Dec 02 '10 at 23:17
0

getInstance() is used ALL over the place in the Zend Framework, which is my goto for standards and conventions in code.

as for the create(), what about using the magic __construct method so that when you do new Blah() it calls the __construct method for that class?

Bob Baddeley
  • 2,264
  • 1
  • 16
  • 22
  • I set `__construct` to private (or protected when dealing with inheritance) and `return new self($args);` from `create()` – Dan Lugg Nov 30 '10 at 06:11
  • so @Tomcat, wouldn't that create two instances of your object? $object1 = new Blah(). $object2 = $object1->create(); What advantage does your method offer? – Bob Baddeley Nov 30 '10 at 06:20
  • Aside from allowing instance-less method chaining (`Class::create()->method()->method();`), none. I have an odd hangup about mixing the `new` keyword with instance retrieval methods in the same scope. – Dan Lugg Nov 30 '10 at 06:26
  • it encapsulates the construction of the object. If he changes the object later (e.g. from Foo to Bar) he only needs to change the factory method, not every single new Foo() invocation; this is definitely a good thing. In this case, it implements a singleton; whether that's good or not is subject to debate. – El Yobo Nov 30 '10 at 06:58
0

You should user __construct method then using create method. As __construct is called by itself you can do initialization and other things in constructor . Another benefit is you can forget to call create() method and your object can be in inconsistent state

shashuec
  • 684
  • 8
  • 20
  • I don't forget :) Also, I often do this; `if($obj = Class::create()){`, therefore providing conditions of instantiation failure, but perhaps this could better be left to Exceptions? – Dan Lugg Nov 30 '10 at 06:27
  • The point of factory methods is to provide a consistent way to instantiate objects and to easily allow you to substitute a different object if necessary. If you use $foo = new Bar(), then if you want to change all Bar objects to a Blarg, you need to replace every new Bar() statement. If you use a factory, you only replace one. So, in short, what you're doing is already good, disregard Shashwat. – El Yobo Nov 30 '10 at 06:56