On Tue, May 8, 2012 at 3:06 PM, Marc Delisle <marc@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@gmail.com> wrote:
>
>>
>>
>> On Mon, May 7, 2012 at 6:24 PM, Rouslan Placella <rouslan@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@gmail.com>  wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, May 3, 2012 at 4:38 PM, Michal Èihaø<michal@cihar.com>  wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Dne Tue, 1 May 2012 02:02:38 +0530
>>>>>> Chanaka Dharmarathna<pe.chanaka.ck@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@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@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@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
____________________________________