[Phpmyadmin-devel] Clarification on Design of PMA_RecentFavoriteTable Class
Hi, It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic. And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ? Regards ! -- Chanaka Dharmarathna *http://chanakaindrajith.blogspot.com/*<http://chanakaindrajith.blogspot.com/>
Hi, I thought we have these code can avoid errors, in my new PR I improve it. <pre> /** * Returns class instance. * * @param string $type the table type * * @return PMA_RecentFavoriteTable */ public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } else { if (self::$_instance[$type]->table_type != $type) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } } return self::$_instance[$type]; } </pre> Does my PR fix it? 2014-04-17 0:02 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
Hi,
It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic.
And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ?
Regards ! -- Chanaka Dharmarathna http://chanakaindrajith.blogspot.com/
------------------------------------------------------------------------------ Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/NeoTech _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
-- Edward Cheng
On Thu, Apr 17, 2014 at 9:47 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, I thought we have these code can avoid errors, in my new PR I improve it. <pre> /** * Returns class instance. * * @param string $type the table type * * @return PMA_RecentFavoriteTable */ public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } else { if (self::$_instance[$type]->table_type != $type) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } } return self::$_instance[$type]; } </pre> Does my PR fix it?
2014-04-17 0:02 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
Hi,
It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic.
And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ?
Hi Edward, Your code does not avoid creating many instances. By the way, we better first clarify our self, the requirement. Any thoughts ? Regards ! -- Chanaka Dharmarathna *http://chanakaindrajith.blogspot.com/*<http://chanakaindrajith.blogspot.com/>
Hi, Chanaka Dharmarathna, I'm sorry code I provided have problems, see sample blow, I fixed them <pre> <?php header("Content-type:text/plain"); class Sample { private $selfData; private $selfType; private static $_instance; public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new Sample($type); } else { if (! array_key_exists($type, self::$_instance)) { self::$_instance[$type] = new Sample($type); } } return self::$_instance[$type]; } public function __construct($type) { $this->selfType = $type; } public function setData($data) { $this->selfData = $data; } public function getData() { return $this->selfData; } } Sample::getInstance('A')->setData('AData'); echo Sample::getInstance('A')->getData(); Sample::getInstance('B')->setData('BData'); echo Sample::getInstance('A')->getData(); echo Sample::getInstance('B')->getData(); </pre> Results is "ADataADataBData", is that ok? 2014-04-18 1:01 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
On Thu, Apr 17, 2014 at 9:47 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, I thought we have these code can avoid errors, in my new PR I improve it. <pre> /** * Returns class instance. * * @param string $type the table type * * @return PMA_RecentFavoriteTable */ public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } else { if (self::$_instance[$type]->table_type != $type) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } } return self::$_instance[$type]; } </pre> Does my PR fix it?
2014-04-17 0:02 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
Hi,
It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic.
And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ?
Hi Edward,
Your code does not avoid creating many instances. By the way, we better first clarify our self, the requirement. Any thoughts ?
Regards ! -- Chanaka Dharmarathna http://chanakaindrajith.blogspot.com/
------------------------------------------------------------------------------ Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/NeoTech _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
-- Edward Cheng
Hi Edward, On Fri, Apr 18, 2014 at 6:26 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, Chanaka Dharmarathna,
I'm sorry code I provided have problems, see sample blow, I fixed them <pre> <?php header("Content-type:text/plain");
class Sample {
private $selfData; private $selfType;
private static $_instance;
public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new Sample($type); } else { if (! array_key_exists($type, self::$_instance)) { self::$_instance[$type] = new Sample($type); } } return self::$_instance[$type]; }
Nice try. This logic seems to be fine, since there are only two new objects are creating. shall we do following things to the current code. 1. Modify $_instance name to $_instances (or better; $_instanceArray ? ) 2. Initialize it to empty array so that developers will not confuse. (normally singleton pattern keep single instance, but here it's array) 3. Apply above logic. (instead null check use array empty check since step 2) 4. Make constructor and $_instances array private. there are more variables you can make private.
public function __construct($type) { $this->selfType = $type; } public function setData($data) { $this->selfData = $data; } public function getData() { return $this->selfData; } }
Sample::getInstance('A')->setData('AData'); echo Sample::getInstance('A')->getData(); Sample::getInstance('B')->setData('BData'); echo Sample::getInstance('A')->getData(); echo Sample::getInstance('B')->getData();
</pre>
Results is "ADataADataBData", is that ok?
Result is obvious. I was worrying about the requests from different users. But seems web server is managing it to run requests sequentially, which gathered into a queue.
2014-04-18 1:01 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
On Thu, Apr 17, 2014 at 9:47 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, I thought we have these code can avoid errors, in my new PR I improve
it.
<pre> /** * Returns class instance. * * @param string $type the table type * * @return PMA_RecentFavoriteTable */ public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } else { if (self::$_instance[$type]->table_type != $type) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } } return self::$_instance[$type]; } </pre> Does my PR fix it?
2014-04-17 0:02 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com :
Hi,
It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic.
And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ?
Hi Edward,
Your code does not avoid creating many instances. By the way, we better first clarify our self, the requirement. Any thoughts ?
Btw, if you are not posting inline, please use bottom posting ;) Regards ! -- Chanaka Dharmarathna *http://chanakaindrajith.blogspot.com/*<http://chanakaindrajith.blogspot.com/>
Hi, Chanaka 2014-04-19 0:32 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
Hi Edward,
On Fri, Apr 18, 2014 at 6:26 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, Chanaka Dharmarathna,
I'm sorry code I provided have problems, see sample blow, I fixed them <pre> <?php header("Content-type:text/plain");
class Sample {
private $selfData; private $selfType;
private static $_instance;
public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new Sample($type); } else { if (! array_key_exists($type, self::$_instance)) { self::$_instance[$type] = new Sample($type); } } return self::$_instance[$type]; }
Nice try. This logic seems to be fine, since there are only two new objects are creating. shall we do following things to the current code.
1. Modify $_instance name to $_instances (or better; $_instanceArray ? ) 2. Initialize it to empty array so that developers will not confuse. (normally singleton pattern keep single instance, but here it's array) 3. Apply above logic. (instead null check use array empty check since step 2) Using is_null() is ok, because logic is "Var exist but empty, now function is going to assign it" 4. Make constructor and $_instances array private. there are more variables you can make private. Yeah, I fixed them :)
public function __construct($type) { $this->selfType = $type; } public function setData($data) { $this->selfData = $data; } public function getData() { return $this->selfData; } }
Sample::getInstance('A')->setData('AData'); echo Sample::getInstance('A')->getData(); Sample::getInstance('B')->setData('BData'); echo Sample::getInstance('A')->getData(); echo Sample::getInstance('B')->getData();
</pre>
Results is "ADataADataBData", is that ok?
Result is obvious. I was worrying about the requests from different users. But seems web server is managing it to run requests sequentially, which gathered into a queue.
2014-04-18 1:01 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
On Thu, Apr 17, 2014 at 9:47 AM, Edward Cheng <c4150221@gmail.com> wrote:
Hi, I thought we have these code can avoid errors, in my new PR I improve it. <pre> /** * Returns class instance. * * @param string $type the table type * * @return PMA_RecentFavoriteTable */ public static function getInstance($type) { if (is_null(self::$_instance)) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } else { if (self::$_instance[$type]->table_type != $type) { self::$_instance[$type] = new PMA_RecentFavoriteTable($type); } } return self::$_instance[$type]; } </pre> Does my PR fix it?
2014-04-17 0:02 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck@gmail.com>:
Hi,
It seems like, the latest PMA_RecentFavoriteTable is trying to behave as singleton design pattern. Is that a requirement ? I mean is there any reason for making it singleton ? By the way, there is good possibility to create new instances again and again with current logic.
And the class has $table_type instance variable and more. Isn't it risky behaviour to share a static instance throughout the web server, which has instance variable which used in functions of that instance ?
Hi Edward,
Your code does not avoid creating many instances. By the way, we better first clarify our self, the requirement. Any thoughts ?
Btw, if you are not posting inline, please use bottom posting ;) Thank you very much!
Regards ! -- Chanaka Dharmarathna http://chanakaindrajith.blogspot.com/
------------------------------------------------------------------------------ Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/NeoTech _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
-- Edward Cheng
participants (2)
-
Chanaka Dharmarathna -
Edward Cheng