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.
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").