Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.
Popular code excerpts may be moved to "Code Snippets" by the moderators.
matthijs wrote:Seems better to place the query outside the loop. First get all results you need. Then loop through the result array to show what you want
What will be the advantages?
There are 10 types of people in this world, those who understand binary and those who don't
when you implemented my version of the code, you changed $i back to 0 up the top. If you follow it counting, you'll realise its wrong, that why I set it at 1.
<table>
<?
$cols = 3;
$i = 3;
while ($rows[] = @mysql_fetch_array($sql_result_types)) { }
foreach($rows as $row)
{
if !($i % $cols)
echo "<tr>";
echo "<td>" . $row['name'] . "</td>";
$i++;
if !($i % $cols)
echo "</tr>";
}
// add this portion outside so that if there isn't an exact multiple of $cols, it will still close the table row
if ($i % $cols)
echo "</tr>";
?>
</table>
<table>
<?
$cols = 3;
$i = 3;
while ($rows[] = @mysql_fetch_array($sql_result_types)) { }
foreach($rows as $row)
{
if !($i % $cols)
echo "<tr>";
echo "<td>" . $row['name'] . "</td>";
$i++;
if !($i % $cols)
echo "</tr>";
}
// add this portion outside so that if there isn't an exact multiple of $cols, it will still close the table row
if ($i % $cols)
echo "</tr>";
?>
</table>
By using this code, you are now causing your program runtime to increase exponentially at O(n^2) from having 2 loops as opposed to having 1 loop (which was posted earlier by VladSun). In this example, there is no reason to have that extra runtime overhead addition to the extra memory consumption from caching all of the names in the $rows table.
programmingjeff wrote:
By using this code, you are now causing your program runtime to increase exponentially at O(n^2) from having 2 loops as opposed to having 1 loop (which was posted earlier by VladSun). In this example, there is no reason to have that extra runtime overhead addition to the extra memory consumption from caching all of the names in the $rows table.
I really doubt you are going to notice any difference whatsoever by changing this to one loop.
On second thought - if I am to follow the MVC style I'll have to do this like programmingjeff said in order to have the M-V-C separation... What do you think?
(the posting is a little offtopic, but I am very interested in getting some answers.)
There are 10 types of people in this world, those who understand binary and those who don't