[Phpmyadmin-devel] Refactoring: Displaying query results

Chanaka Dharmarathna pe.chanaka.ck at gmail.com
Tue May 8 10:57:38 CEST 2012 

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 !
-- 
____________________________________

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/551bd5a0/attachment.html>

More information about the Developers mailing list