mjpearson / Pandra

Cassandra abstraction layer and keyspace scaffolder for PHP developers --- ABANDONED.
GNU Lesser General Public License v3.0
93 stars 11 forks source link

Implement PandraCore as a singleton #24

Closed JordanP closed 14 years ago

JordanP commented 14 years ago

Most methods of PandraCore are static, and those which are not static are used in a static way (PandraCore::getCFSlice, PandraCore::getCFSliceMulti).

IMHO there is no future need to instantiate PandraCore and, at least, no need to instantiate it several time.

To implement the singleton pattern, just add the following code before static public function getSupportedModes

private static $_instance = NULL;

private function __construct() {
}

public static function getInstance() 
{
    if (!is_null(self::$instance)) {
        $c = __CLASS__;
        self::$_instance = new $c;
    }

    return self::$_instance;
}
mjpearson commented 14 years ago

The functions you mentioned which are used statically, should be static public anyway (dev oversight, haven't looked at those methods for a while). Core itself can't be serialised etc. and there's no plan to support that behaviour. I'll rework core to declare those public methods as static, with the fixes in your other ticket.

Can you think of any use cases where a singleton interface would be needed?

JordanP commented 14 years ago

If you plan to make all methods static in Core, than this class should'nt be instantiate at all. The "goal" of static methods is to be able to call them, from everywhere in Pandra, without having an instance.

I think using a singleton would make people use PandraCore in the right way. (at least forbird them to instantiate the class several time).

IanRogers commented 14 years ago

I'd like to see these methods moved away from being class statics and into singletons.

I'd prefer to use a singleton for each unique ClusterSpec/Keyspace pair rather than call setActivePool($keySpace) - particularly as setActivePool() doesn't have a notion of seperate cassandra clusters.