[Phpmyadmin-devel] GSOC 2012 - Refactoring: Insert/edit, Privileges, Operations, Structure

Sat Mar 24 16:19:41 CET 2012

On Sat, Mar 24, 2012 at 7:13 PM, prakul agarwal
<discover.prakul at gmail.com>wrote:

> Hello sir,
>
> Im new to PHP code refactoring. Going through this thread i worked out for
> tbl_structure.php (line 199-209):
>
>  <?php
>     $a
> =array("column","type","collation","attributes","null","default","extra","view");
>     $b =
> array("Name","Type","Collation","Attributes","Null","Default","Extra","View");
>
>     for($i=0;$i<8;$i++)
>     {
>       echo " <th id=\"th$i\" class=\"$a[$i]\" > __('$b[$i]') </th>" ;
>
>     }
>     ?>
>
>     This code has the same output as the already existing code, plus this
> removes redundancy and makes maintenance easier. Does this qualifies as
> good refactoring?
>
> thank you.
>
>
>
> On 24 March 2012 18:31, Marc Delisle <marc at infomarc.info> wrote:
>
>> Le 2012-03-24 01:18, Thilina Buddika Abeyrathna a écrit :
>> > On Fri, Mar 23, 2012 at 8:28 PM, Thilina Buddika Abeyrathna <
>> > thilinaabeyrathna at gmail.com> wrote:
>> >
>> >>
>> >>
>> >> On Fri, Mar 23, 2012 at 5:26 PM, Marc Delisle <marc at infomarc.info>
>> wrote:
>> >>
>> >>> Le 2012-03-22 21:57, Thilina Buddika Abeyrathna a écrit :
>> >>>> On Sat, Mar 17, 2012 at 9:30 PM, Thilina Buddika Abeyrathna <
>> >>>> thilinaabeyrathna at gmail.com> wrote:
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>> On Sat, Mar 17, 2012 at 8:38 PM, Marc Delisle <marc at infomarc.info>
>> >>> wrote:
>> >>>>>
>> >>>>>> Le 2012-03-17 10:30, Thilina Buddika Abeyrathna a écrit :
>> >>>>>>> Hi Marc,
>> >>>>>>> I did a feature request and uploaded the patch file [0] to
>> >>> Sourceforge.
>> >>>>>> But
>> >>>>>>> it is relevant to Context. Therefor I think it will review after
>> the
>> >>>>>>> deadline. So I like to much familiar with that Refactoring
>> project.
>> >>>>>>> When I implementing above feature I went through the
>> tbl_change.php
>> >>>>>> file. I
>> >>>>>>> need to familiar with other files also which is relevant to above
>> >>>>>> project.
>> >>>>>>> So  if you have any idea, please let me know, I am grateful if I
>> >>> could
>> >>>>>> get
>> >>>>>>> to know your suggestions.
>> >>>>>>
>> >>>>>> Hi Thilina,
>> >>>>>> I have reviewed your patch.
>> >>>>>>
>> >>>>>> About refactoring, I have an idea. In the master branch,
>> >>>>>> tbl_structure.php, look at lines 199-209. Can you suggest some
>> >>>>>> refactoring of these lines?
>> >>>>>>
>> >>>>>> Hi,
>> >>>>> Thanks Marc. I'll look at that file.
>> >>>>>
>> >>>>>
>> >>>> Hi Marc,
>> >>>>
>> >>>> Sorry for the delay to reply. I had a mid semester exam. So I look at
>> >>> the
>> >>>> tbl_structure.php file and I have some suggestion for refactoring of
>> >>> line
>> >>>> 199-209.
>> >>>> First thing is  it is better to have an underscore for id field
>> >>>>     <th id="th_<?php echo ++$i; ?>"></th>
>> >>>>
>> >>>> Second one is,
>> >>>> when using the if condition or for loop with html, it is better to
>> use
>> >>> this
>> >>>> style
>> >>>>
>> >>>> <php? if(condition) : ?>
>> >>>>     <html>
>> >>>>     <html>
>> >>>> <?php endif; ?>
>> >>>
>> >>> Thilina,
>> >>> In your suggestions, I don't see any refactoring techniques; yours are
>> >>> mostly about syntax. See [0] and [1] for references.
>> >>>
>> >>> Also, I don't believe we use this "if" syntax in the current codebase.
>> >>>
>> >>> [0] http://en.wikipedia.org/wiki/Refactoring
>> >>> [1] http://devzone.zend.com/1067/refactoring-php-code/
>> >>>
>> >>>
>> >>> Thank you Marc. I will refer those articles.
>> >>
>> >> Hi Marc,
>> >
>> > In this code snippet, id attribute is not make any sense, I meant, id =
>> > th1, id = th2, etc. But class attribute is make sense. I checked the css
>> > file and js file, And I couldn't find any place where this class values
>> > used. I think it is better to change the id attribute values to meaning
>> > full one. as an example,
>> > <th id = "column"
>> > And can we use the same class for all this 'th' ? (line 199-209)
>>
>>
>> Thilina,
>> there must be a reason to use the "id" attribute; in this case I could
>> not find a reason so I removed them.
>>
>> Hi Marc,
I didn't mean that removing id attribute. I meant value of id attribute
(eg. th1, th2). Sorry for the mis communication.

>  These class attributes were not used, except for the "action" class
>> which is used in functions.js, displayMoreTableOpts(). There is no
>> reason to use the same class for all these 'th', if we do nothing with
>> the class, so I removed them (keeping "action").
>>
>> Anyway, thank you for your comment.

>  --
>> Marc Delisle
>> http://infomarc.info
>>
>>
>> ------------------------------------------------------------------------------
>> This SF email is sponsosred by:
>> Try Windows Azure free for 90 days Click Here
>> http://p.sf.net/sfu/sfd2d-msazure
>> _______________________________________________
>> Phpmyadmin-devel mailing list
>> Phpmyadmin-devel at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>>
>
>
>
> --
> Prakul Agarwal,
> Indian Institute of Information Technology, Allahabad
>
>
>
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> Phpmyadmin-devel mailing list
> Phpmyadmin-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>
>

-- 
Regards.

Thilina Buddika Abeyrathna,
Department of Computer Engineering,
Faculty Of Engineering,
University of Peradeniya,
Sri Lanka.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120324/ef63dc24/attachment.html>