[Phpmyadmin-devel] Refactoring: Displaying query results

Tue May 8 11:36:02 CEST 2012

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