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)

--
Regards.

Thilina Buddika Abeyrathna,
Department of Computer Engineering,
Faculty Of Engineering,
University of Peradeniya,
Sri Lanka.