Op 24 maart 2012 16:26 heeft Marc Delisle marc@infomarc.info het volgende geschreven:
Le 2012-03-24 09:43, prakul agarwal a écrit :
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.
Hi, yes, this is good refactoring, but better refactoring would be:
- avoid generic names like $a, $b
- avoid using the constant 8, instead counting the elements in the array
Using a loop, like you did, is what I had in mind when I proposed this challenge. Technically, after refactoring, the code had to produce the same output, as I suspect your example does.
When using a single array instead of one (f.e. a key => value pair in this case, because there are only two lists, but there are other options), you can use a foreach. This has several advantages :
- no need to introduce $i - no need for the $i < count($array) to end the for loop. - foreach loop is better readable than a for loop. - the class name and caption are grouped in one array, making it easier to add a new column later on (with less change of a mistake, because only one array needs to be changed)
In this discussion, however, code improvement was gained not just by refactoring, but by examining the reasons behing the code and simplifying it.
-- 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@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel