Hello sir,<div> </div><div>Im new to PHP code refactoring. Going through this thread i worked out for tbl_structure.php (line 199-209):</div><div><br></div><div> <?php</div><div> $a =array("column","type","collation","attributes","null","default","extra","view");</div>
<div> $b = array("Name","Type","Collation","Attributes","Null","Default","Extra","View");</div><div> </div><div> for($i=0;$i<8;$i++)</div>
<div> {</div><div> echo " <th id=\"th$i\" class=\"$a[$i]\" > __('$b[$i]') </th>" ;</div><div> </div><div> }</div><div> ?></div><div> </div><div> 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?</div>
<div><br></div><div>thank you.</div><div> </div><div> <br><br><div class="gmail_quote">On 24 March 2012 18:31, Marc Delisle <span dir="ltr"><<a href="mailto:marc@infomarc.info">marc@infomarc.info</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Le 2012-03-24 01:18, Thilina Buddika Abeyrathna a écrit :<br>
> On Fri, Mar 23, 2012 at 8:28 PM, Thilina Buddika Abeyrathna <<br>
> <a href="mailto:thilinaabeyrathna@gmail.com">thilinaabeyrathna@gmail.com</a>> wrote:<br>
><br>
>><br>
>><br>
>> On Fri, Mar 23, 2012 at 5:26 PM, Marc Delisle <<a href="mailto:marc@infomarc.info">marc@infomarc.info</a>> wrote:<br>
>><br>
>>> Le 2012-03-22 21:57, Thilina Buddika Abeyrathna a écrit :<br>
>>>> On Sat, Mar 17, 2012 at 9:30 PM, Thilina Buddika Abeyrathna <<br>
>>>> <a href="mailto:thilinaabeyrathna@gmail.com">thilinaabeyrathna@gmail.com</a>> wrote:<br>
>>>><br>
>>>>><br>
>>>>><br>
>>>>> On Sat, Mar 17, 2012 at 8:38 PM, Marc Delisle <<a href="mailto:marc@infomarc.info">marc@infomarc.info</a>><br>
>>> wrote:<br>
>>>>><br>
>>>>>> Le 2012-03-17 10:30, Thilina Buddika Abeyrathna a écrit :<br>
>>>>>>> Hi Marc,<br>
>>>>>>> I did a feature request and uploaded the patch file [0] to<br>
>>> Sourceforge.<br>
>>>>>> But<br>
>>>>>>> it is relevant to Context. Therefor I think it will review after the<br>
>>>>>>> deadline. So I like to much familiar with that Refactoring project.<br>
>>>>>>> When I implementing above feature I went through the tbl_change.php<br>
>>>>>> file. I<br>
>>>>>>> need to familiar with other files also which is relevant to above<br>
>>>>>> project.<br>
>>>>>>> So if you have any idea, please let me know, I am grateful if I<br>
>>> could<br>
>>>>>> get<br>
>>>>>>> to know your suggestions.<br>
>>>>>><br>
>>>>>> Hi Thilina,<br>
>>>>>> I have reviewed your patch.<br>
>>>>>><br>
>>>>>> About refactoring, I have an idea. In the master branch,<br>
>>>>>> tbl_structure.php, look at lines 199-209. Can you suggest some<br>
>>>>>> refactoring of these lines?<br>
>>>>>><br>
>>>>>> Hi,<br>
>>>>> Thanks Marc. I'll look at that file.<br>
>>>>><br>
>>>>><br>
>>>> Hi Marc,<br>
>>>><br>
>>>> Sorry for the delay to reply. I had a mid semester exam. So I look at<br>
>>> the<br>
>>>> tbl_structure.php file and I have some suggestion for refactoring of<br>
>>> line<br>
>>>> 199-209.<br>
>>>> First thing is it is better to have an underscore for id field<br>
>>>> <th id="th_<?php echo ++$i; ?>"></th><br>
>>>><br>
>>>> Second one is,<br>
>>>> when using the if condition or for loop with html, it is better to use<br>
>>> this<br>
>>>> style<br>
>>>><br>
>>>> <php? if(condition) : ?><br>
>>>> <html><br>
>>>> <html><br>
>>>> <?php endif; ?><br>
>>><br>
>>> Thilina,<br>
>>> In your suggestions, I don't see any refactoring techniques; yours are<br>
>>> mostly about syntax. See [0] and [1] for references.<br>
>>><br>
>>> Also, I don't believe we use this "if" syntax in the current codebase.<br>
>>><br>
>>> [0] <a href="http://en.wikipedia.org/wiki/Refactoring" target="_blank">http://en.wikipedia.org/wiki/Refactoring</a><br>
>>> [1] <a href="http://devzone.zend.com/1067/refactoring-php-code/" target="_blank">http://devzone.zend.com/1067/refactoring-php-code/</a><br>
>>><br>
>>><br>
>>> Thank you Marc. I will refer those articles.<br>
>><br>
>> Hi Marc,<br>
><br>
> In this code snippet, id attribute is not make any sense, I meant, id =<br>
> th1, id = th2, etc. But class attribute is make sense. I checked the css<br>
> file and js file, And I couldn't find any place where this class values<br>
> used. I think it is better to change the id attribute values to meaning<br>
> full one. as an example,<br>
> <th id = "column"<br>
> And can we use the same class for all this 'th' ? (line 199-209)<br>
<br>
<br>
Thilina,<br>
there must be a reason to use the "id" attribute; in this case I could<br>
not find a reason so I removed them.<br>
<br>
These class attributes were not used, except for the "action" class<br>
which is used in functions.js, displayMoreTableOpts(). There is no<br>
reason to use the same class for all these 'th', if we do nothing with<br>
the class, so I removed them (keeping "action").<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Marc Delisle<br>
<a href="http://infomarc.info" target="_blank">http://infomarc.info</a><br>
<br>
------------------------------------------------------------------------------<br>
This SF email is sponsosred by:<br>
Try Windows Azure free for 90 days Click Here<br>
<a href="http://p.sf.net/sfu/sfd2d-msazure" target="_blank">http://p.sf.net/sfu/sfd2d-msazure</a><br>
_______________________________________________<br>
Phpmyadmin-devel mailing list<br>
<a href="mailto:Phpmyadmin-devel@lists.sourceforge.net">Phpmyadmin-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel" target="_blank">https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>Prakul Agarwal,<div>Indian Institute of Information Technology, Allahabad<br><br></div><br>
</div>