[Phpmyadmin-devel] Clarification on Design of PMA_RecentFavoriteTable Class

[Edward Cheng]

Hi, Chanaka

2014-04-19 0:32 GMT+08:00 Chanaka Dharmarathna <pe.chanaka.ck at gmail.com>:
> Hi Edward,
>
> On Fri, Apr 18, 2014 at 6:26 AM, Edward Cheng <c4150221 at 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 at gmail.com>:
>> > On Thu, Apr 17, 2014 at 9:47 AM, Edward Cheng <c4150221 at 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 at 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>



-- 
Edward Cheng