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

Sat Mar 24 14:43:17 CET 2012

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.
>
> 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").
>
> --
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120324/e03fa5ab/attachment.html>