simple singleton class

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

potatobob
Forum Newbie
Posts: 9
Joined: Thu Nov 23, 2006 3:34 pm

simple singleton class

Post by potatobob »

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


I feel like this can be done better or something is missing or wrong

Code: Select all

class SpotSec_Singleton
{
	/**
	 * Stores instances
	 *
	 * @var array
	 */
	static private $_instances = array();

	/**
	 * Singleton Pattern
	 */
	private function __construct()
	{}

	/**
	 * Returns an instance of a class. $instanceName can be used to create
	 * multitons.
	 *
	 * @param string $class
	 * @param string $instanceName
	 * @return class
	 */
	static public function &getInstance($class, $instanceName = null)
	{
		$instance =& self::$_instances[$instanceName][$class];

		if (!isset($instance) && !$instance instanceof $class) {
			$instance = new $class;
		}

		return $instance;
	}

	/**
	 * Removes an instance of a class
	 *
	 * @param string $class
	 * @param string $instanceName
	 */
	static public function removeInstance($class, $instanceName = null)
	{
		$instance =& self::$_instances[$instanceName][$class];

		if (isset($instance) && $instance instanceof $class) {
			unset(self::$_instances[$instanceName][$class]);
		}
	}
}

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

You're allowing multiple singleton instances? ... it's not a singleton then. :? It would appear you want a registry.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

You seem to have over-complicated things ;)

It's not a singleton, it's some sort of self-registering factory thingy-majiggy :twisted:

Is the aim of this to store a single instance of any object you ask it to create? What about constructor parameters?

Like feyd says, the registry exists for this reason. It's more sensible to do:

Code: Select all

if (!Registry::instance()->has("className"))
{
    Registry::instance()->register(new ClassName( ... ), "className");
}
$obj = Registry::instance()->get("className");
Or something similar.

EDIT | "public static" not "static public" ;)
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

How would we define a singleton anyway? Even if your script instance allowed only one instance of something to exist... It would still be possible that another instance exists in another thread (where the same php script is executed...) Personally i don't see much use for singletons in day to day php development...
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

timvw wrote:How would we define a singleton anyway? Even if your script instance allowed only one instance of something to exist... It would still be possible that another instance exists in another thread (where the same php script is executed...) Personally i don't see much use for singletons in day to day php development...
I would agree with this, most often if your applications are designed properly there is little to no need for singletons.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

I'd third that.
The only time I've used a singleton is when I wanted to use a class to group together a bunch of functions i.e. no state whatsoever. Even then I use a static class which avoids all the getInstance() malarkey.
jmut
Forum Regular
Posts: 945
Joined: Tue Jul 05, 2005 3:54 am
Location: Sofia, Bulgaria
Contact:

Post by jmut »

I would not agree with singleton is of no use in day to day php.
Singleton is necessary to represent the logic behind.... we need one incance of this (as a business constraint).
Of course if you are single developer you can just create one instance(you know about it), put it in a registry and all will be fine.
The idea is to appropriately represent the business logic (the domain environment)....so if another developer steps in....he will not mistakanly make
two or more instances (he will have to figure out, with docs hopefully, why only one instance is necessary), else he/she might decide doing more instances as at first glance it seems to make perfect sense. Examples I could come up with are... db connection, logging class, smarty instance(a bit specific but you get the idea), some controller class(after all there should be one boss controlling the work flow :))


This is the simplest singleton I thought of.

Code: Select all

<?php

class Logger {
    static private $instance = NULL;

    /**
     * Constructor() and clone() are private !!!
     * This is done so that a developer can't mistakenly
     * create a second instance of the  Logger class using
     * the  new or  clone operators; therefore,  getInstance()
     * is the only way to access the singleton class instance.
     */
    private function __construct() {}
    private function __clone() { }

    static function getInstance()
    {
        /**
         * If instance is already defined it returns the created instance.
         */
        if (self::$instance == NULL) {
            self::$instance = new self();
        }
        return self::$instance;
    }

    public function log($str)
    {
        // Take care of logging
        echo "logging: <<{$str}>>";
    }

}

Logger::getInstance()->log("Checkpoint");

?>
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Why does Logger need to be a singleton?
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

ole wrote:Why does Logger need to be a singleton?
Logger may need an exclusive hold on the file it writes to if it's a flat-file logger (i.e. using flock(()). Only a singleton will work here... additionally, it helps when you come to dump the contents of the log for that session's data.

A registry could be used (they almost always can) but it would be less convenient IMO.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

ole wrote:Why does Logger need to be a singleton?
This is an interesting question because it gets at one point of confusion about the Singleton pattern. There is a difference between having only an single instance of an object and having only a single instance of a datasource. In the example of a Logger, or something like a database connection class, it may not matter that there are multiple instances of the object -- only that they are read/write properly to a single datasource.

I think that in most cases when programmers think "Gee I like one of them Singeltons!" they really want either a shared datasource or a Registry. These are both dancing around the very basic OO concept of a Factory.
(#10850)
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

d11wtq wrote:
ole wrote:Why does Logger need to be a singleton?
Logger may need an exclusive hold on the file it writes to if it's a flat-file logger (i.e. using flock(()). Only a singleton will work here... additionally, it helps when you come to dump the contents of the log for that session's data.

A registry could be used (they almost always can) but it would be less convenient IMO.
OK but logger is just echoing. Here's a solution to your problem d

Code: Select all

class Logger
{
    private static $_openFiles = array();
    private $_fHandle, $_file;
    public function __construct($file = null)
    {
        if ($file) {
            $this->openFile($file);
        }
    }
    public function __destruct()
    {
        $this->closeFile(); // release
    }
    public function openFile($file)
    {
        if (isset(self::$_openFiles[$file])) { // is file already in use?
            throw new Exception('File ' . $file . ' already has a logger instance');
        }
        if (!$fHandle = fopen($file, 'w')) {
            return false;
        }
        $this->_fHandle = $fHandle;
        $this->_file = $file;
        self::$_openFiles[$file] = true; // file is now in use
        return true;
    }
    public function closeFile()
    {
        fclose($this->_fHandle);
        self::$_openFiles[$this->_file] = null; // file is no longer in use
    }
    public function log($str)
    {
        if (!$this->_fHandle) {
            throw new Exception('Cannot log no file open');
        }
        return fwrite($this->_fHandle, $str);
    }
    public function getFile()
    {
        return $this->_file;
    }
}
No singleton! :)
jmut
Forum Regular
Posts: 945
Joined: Tue Jul 05, 2005 3:54 am
Location: Sofia, Bulgaria
Contact:

Post by jmut »

arborint wrote:
ole wrote:Why does Logger need to be a singleton?
....There is a difference between having only an single instance of an object and having only a single instance of a datasource. In the example of a Logger, or something like a database connection class, it may not matter that there are multiple instances of the object -- only that they are read/write properly to a single datasource....
It is true that it might not be a problem as long as you use same datasource but still....How about using transactions? You sure cannot use more than one db connection instance because you will loose the transaction despide the fact you use same datasource to write to.
jmut
Forum Regular
Posts: 945
Joined: Tue Jul 05, 2005 3:54 am
Location: Sofia, Bulgaria
Contact:

Post by jmut »

I don't know how the explain what I mean. IMO the idea behind using the design patterns and singleton in particular is to constraint to developer to the limits dictated by the business/domain logic. I mean..... he/she should not be able to do wrong stuff (in this case having multiple of something) ..period.

So simple registry will just not do it because all registry does is keep an instance....But this will not prevent a developer from cloning the object kept in the registry and hence breaking the rule...having to objects which can be misused. No, provided this oppertunity is already bad stuff...smelly code!

Ultimately this is also the idea behind property/method visibility. When property is private you just cannot misuse it...hence you have a more strict environment to work in, less combinations/things to think about and in a result easier programming.

Generally speaking you don't need any of this stuff to make a great application (with lots of fancy great/speedy functionality). Developer can easily write a documentation or just have some constraint in mind...and never misused it. He will not use the property...he will not make multiple instances...but this is just too much to think about...so the idea is to present on code level all of these constarints.

Nowadays great application is considered application easy to read/modify/extend/refactor or whatever......and all those to happen we need those business constraints implemented in code. So that developers have a solid ground to work on ....and less opportunities to break the code (less things to think about..but concentrate on problem).

What singleton provide is just a way to implement such constraints. So that whatever the programmer does....there is one and no more instances. Of course he can build another class and hence breaking the rule...but this is whole other matter already.
Hopefully this will make more sense now.

P.S. I don't know if the Logger class in particular is a good example of singleton it depends on the business constraints...environment etc. But it as well might be a good candidate
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

jmut: You are completely right about need to restrict to one instance in some cases. The point I wish to make is a lot of developers overuse singletons or make something a singleton so that they don't have to worry about objects....erm, are you writing object oriented code then? No.

The singleton pattern itself is a bit of a strange one in PHP. Your singleton you posted jmut is undoubtedly a singleton. What about this:

Code: Select all

class Logger
{
    private static $file;
    public static function setFile($file)
    {
        self::$file = $file;
    }
    public static function log($str)
    {
        $h = fopen(self::$file, 'a') or throw new Exception('Couldn\'t open file');
        fwrite($h, $str);
        fclose($h);
    }
    private function __construct() {}
    private function __clone() {}  
}
Would you consider that a singleton? Some might say "it doesn't have a getInstance() method; not a singleton". Those same people would probably say singletons aren't very useful in PHP because static classes can are nearly always better as there's no need for that extra getInstance typing.

In the case of logger making this a proper singleton with a getinstance and a defined private __construct would be better because you can keep the file open and destruct it later but that's not the point here, my point is static classes are used a lot in PHP and your view on frequency of use of singletons in PHP depends of whether you consider static classes to be singletons.
jmut
Forum Regular
Posts: 945
Joined: Tue Jul 05, 2005 3:54 am
Location: Sofia, Bulgaria
Contact:

Post by jmut »

ole: I would consider anything singleton having it provideds the functionality and restrictions singleton provides.
Some might say "it doesn't have a getInstance() method; not a singleton".
Well...obviously this is not true..but on the other hand I think there should be at least minor naming/`code structuring` convention that should be followed.
Design patterns are explained with same examples , same naming convention in each and every book and in some aspect it is understandable that some people are trying to obey those unwritten rules....it is just easier to communitcate the idea/patterns with other devs.
Those same people would probably say singletons aren't very useful in PHP because static classes can are nearly always better as there's no need for that extra getInstance typing.
I think this is same for pretty much each programming language. Most I know of have static classes. Don't see how PHP is special in this.

Now to the question: Would I consider static classes to be singletons.

I personally would not use static classes for following reasons:
- I guess I am more or less following the book...in most of them singletons are not static :)
- I assume there might be a problem using static classes when it comes to a more complex hyerarchy/inheritance. Cannot think of an example right now...but pretty sure none static approach will have more pros than cons.

I totally agree some devs overuse singleton but this is also true for most design patterns.... I personally thing it takes a live time to learn when and how. This is all programming is about for me. Learning syntax of any programming language is the easiest part of it.

I guess we are getting very specific already...so this topic is closed more or less I think. Cheers.
Post Reply