[Phpmyadmin-devel] Refactoring: Displaying query results

Chanaka Dharmarathna pe.chanaka.ck at gmail.com
Tue May 8 15:01:09 CEST 2012 

On Tue, May 8, 2012 at 3:06 PM, Marc Delisle <marc at infomarc.info> wrote:

> Le 2012-05-08 04:57, Chanaka Dharmarathna a écrit :
> > On Mon, May 7, 2012 at 10:07 PM, Chanaka Dharmarathna <
> > pe.chanaka.ck at gmail.com> wrote:
> >
> >>
> >>
> >> On Mon, May 7, 2012 at 6:24 PM, Rouslan Placella <rouslan at placella.com
> >wrote:
> >>
> >>> On 07/05/12 09:27, Chanaka Dharmarathna wrote:
> >>>> On Sat, May 5, 2012 at 1:16 AM, Chanaka Dharmarathna<
> >>>> pe.chanaka.ck at gmail.com>  wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>> On Thu, May 3, 2012 at 4:38 PM, Michal Čihař<michal at cihar.com>
>  wrote:
> >>>>>
> >>>>>> Hi
> >>>>>>
> >>>>>> Dne Tue, 1 May 2012 02:02:38 +0530
> >>>>>> Chanaka Dharmarathna<pe.chanaka.ck at gmail.com>  napsal(a):
> >>>>>>
> >>>>>>> I'm identifying the refactoring points in 'display_tbl.lib.php'
> >>> file. I
> >>>>>> am
> >>>>>>> trying to break the tasks on following areas.
> >>>>>>>
> >>>>>>> * As a library file this should not render the HTML codes in
> >>> functions
> >>>>>> it
> >>>>>>> selves. First, all the HTML content in functions stored in string
> and
> >>>>>>> render at the bottom of the function.
> >>>>>>> Next, return the string having the HTML content, instead of
> >>> rendering it
> >>>>>>> inside the function itself and, from the calling end render that
> >>>>>> content.
> >>>>>>> Probably method names will be modified with adding 'get' phrase.
> >>>>>>>
> >>>>>>> * Some naming conventions are used incorrectly. For method name in
> >>> lib
> >>>>>>> file, naming convention is used generally used as 'PMA_methodName'.
> >>> But
> >>>>>>> some are not following this.
> >>>>>>> While correcting them, suggested conventions in my proposal [0] is
> >>>>>> going to
> >>>>>>> applied.
> >>>>>>>
> >>>>>>> * There are several functions having more than 200 lines. I'm
> >>> intending
> >>>>>> to
> >>>>>>> divide those functions to smaller ones.
> >>> (PMA_displayTableNavigation(),
> >>>>>>> PMA_displayTableHeaders(), PMA_displayTableBody(),
> >>>>>>> PMA_displayVerticalTable(), PMA_displayTable())
> >>>>>>
> >>>>>> I think all this should be rather wrapped in object, so it would be
> >>>>>> PMA_Display::someFunction.
> >>>>>>
> >>>>>> --
> >>>>>>         Michal Čihař | http://cihar.com | http://blog.cihar.com
> >>>>>>
> >>>>>>
> >>>>>>
> >>>
> ------------------------------------------------------------------------------
> >>>>>> Live Security Virtual Conference
> >>>>>> Exclusive live event will cover all the ways today's security and
> >>>>>> threat landscape has changed and how IT managers can respond.
> >>> Discussions
> >>>>>> will include endpoint security, mobile security and the latest in
> >>> malware
> >>>>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> >>>>>> _______________________________________________
> >>>>>> Phpmyadmin-devel mailing list
> >>>>>> Phpmyadmin-devel at lists.sourceforge.net
> >>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>>>>
> >>>>>>
> >>>>> Hi Michal,
> >>>>>
> >>>>> Thanks for your reply. I got the point Michal. After the mentioned
> >>> tasks
> >>>>> plus making this as a class, I can use 'PMA_Display::someFunction'
> >>> where
> >>>>> needed.
> >>>>>
> >>>>>
> >>>>> Regards !
> >>>>> --
> >>>>> ____________________________________
> >>>>>
> >>>>> Chanaka Indrajith
> >>>>> Bsc.Computer Engineering Undergraduate
> >>>>> Faculty of Engineering
> >>>>> University of Peradeniya
> >>>>> Sri Lanka
> >>>>> ____________________________________
> >>>>>
> >>>>>
> >>>> Hi Michal,
> >>>>
> >>>> I am involving with some refactoring in "PMA_displayTableHeaders"
> >>> function
> >>>> in "display_tbl.lib.php" file.
> >>>> As I mentioned in my previous mail, first I'm trying to render all
> HTML
> >>> at
> >>>> the bottom of the function itself.
> >>>> But I'm facing some problems.
> >>>>
> >>>> While current code rendering HTML codes using "echo", there are some
> >>>> function calls which does not return anything but render HTML inside
> >>> that.
> >>>> Ex : "PMA_generate_slider_effect" function in common.lib.php file  ->
> >>> call
> >>>> by 739 line in "display_tbl.lib.php" file.
> >>>>
> >>>> There are several situations like that, in this function. Since they
> >>> have
> >>>> used in other files also (test cases also implemented for them), I do
> >>> not
> >>>> think to change those functions.
> >>>>
> >>>> What I'm going to do is, remove HTML renderings everywhere inside the
> >>>> function, as well as possible by assigning those HTML to a variable,
> and
> >>>> render existing content in the variable before calling that kind of
> >>>> function (as above mentioned).
> >>>>
> >>>> Ex :
> >>>> func {
> >>>>      $tableHeadersHTML .= '';
> >>>>      ----
> >>>>      ----
> >>>>      $tableHeadersHTML .= '<div>'
> >>>>                                     . 'some html content';
> >>>>      ----
> >>>>      *echo $tableHeadersHTML; // render HTML content before another
> >>> function
> >>>> call which not just return a string of HTML*
> >>>>      PMA_generate_slider_effect(params);
> >>>>      ----
> >>>>      // repeat the procedure
> >>>> }
> >>>>
> >>>> Before doing this, I would like to hear your suggestions on this.
> >>>>
> >>>> Regards !
> >>>>
> >>>
> >>> I think that you should rewrite the PMA_generate_slider_effect function
> >>> to output a string, like it should. I'd say don't worry about breaking
> >>> test cases for functions that do things the wrong way around, besides
> >>> those should simple enough to fix. As far as spotting other occurrences
> >>> of the function in the code base, there are only 4 of them in total
> (and
> >>> that includes the function declaration), so there is no excuse not to
> >>> fix all of them. To find every occurrence, run:
> >>>
> >>> git grep PMA_generate_slider_effect
> >>>
> >>> Bye,
> >>> Rouslan
> >>>
> >>>
> >>>
> ------------------------------------------------------------------------------
> >>> Live Security Virtual Conference
> >>> Exclusive live event will cover all the ways today's security and
> >>> threat landscape has changed and how IT managers can respond.
> Discussions
> >>> will include endpoint security, mobile security and the latest in
> malware
> >>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> >>> _______________________________________________
> >>> Phpmyadmin-devel mailing list
> >>> Phpmyadmin-devel at lists.sourceforge.net
> >>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>
> >>
> >> Hi Rouslan,
> >>
> >> That's good idea. Its better to correct those things in good manner.
> >> I'll try to rewrite that kind of functions. As well the tests.
> >>
> >>
> >> Regards !
> >> --
> >> ____________________________________
> >>
> >> Chanaka Indrajith
> >> Bsc.Computer Engineering Undergraduate
> >> Faculty of Engineering
> >> University of Peradeniya
> >> Sri Lanka
> >> ____________________________________
> >>
> >>
> > Hi Rouslan,
> >
> > Do you have any idea of using different naming conventions for this
> > ('PMA_generate_slider_effect') function than general. (not only this,
> there
> > are several functions)
> > Generally function names are like 'PMA_meaningfulDescription()'. This
> does
> > not use underscores after 'PMA_' part and use camel caps.
> > But here it is 'PMA_generate_slider_effect()'. Its different.
> >
> > As well, Since this function is going to return a string after modifying,
> > the name should be change to give a meaning to the user (developer).
> > I'm thinking modify the name with 'get' phrase for that kind of
> functions.
> > Ex :- 'PMA_getSliderEffectGeneratingHtml()' instead of
> > 'PMA_generate_slider_effect' (existing one)
> > Do you have any suggestions on that ?
> >
> > Regards !
>
> PMA_getDivForSliderEffect()
>
> By the way, this is the naming convention for a global function (outside
> a class). Will this be inside a class?
>
> --
> Marc Delisle
> http://infomarc.info
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Phpmyadmin-devel mailing list
> Phpmyadmin-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>

Hi Marc,

That name is seems pretty good. Thanks for suggesting.
And Marc, what about the naming convention of local (for a class) methods.

As I feel common.lib.php file should contain a class and this function will
be inside that.

Regards !
-- 
____________________________________

Chanaka Indrajith
Bsc.Computer Engineering Undergraduate
Faculty of Engineering
University of Peradeniya
Sri Lanka
____________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120508/a4ff719b/attachment.html>

More information about the Developers mailing list