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@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@gmail.com> wrote:
>
>>
>>
>> On Fri, Mar 23, 2012 at 5:26 PM, Marc Delisle <marc@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@gmail.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Sat, Mar 17, 2012 at 8:38 PM, Marc Delisle <marc@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.
Anyway, thank you for your comment.