[Phpmyadmin-devel] Clarification on Design of PMA_RecentFavoriteTable Class

[Chanaka Dharmarathna]

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)
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 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 ;)

Regards !
-- 
Chanaka Dharmarathna
*http://chanakaindrajith.blogspot.com/*<http://chanakaindrajith.blogspot.com/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20140418/f6d9ea28/attachment.html>