[Phpmyadmin-devel] Refactoring: Displaying query results

[Chanaka Dharmarathna]

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
____________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120507/bfab5d71/attachment.html>