Formatting output to specified number of table columns

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.

Moderator: General Moderators

Post Reply
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Formatting output to specified number of table columns

Post by VladSun »

Code: Select all

<table>
	<?php
	$cols = 3;
	$i = 0;
	while ($sql_row = @mysql_fetch_array($sql_result_types))
	{
		if (!($i % $cols))
		{
			if ($i)
			{
				echo "</tr>";
			}
			echo "<tr>";
		}
		echo "<td>".$sql_row['name']."</td>";
		$i++;
	}
	if (($i % $cols) && $i)
	{
		echo "</tr>";
	}
	?>
</table>
This should print the data into a table with $cols columns.

Any better ways to do this?
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
iknownothing
Forum Contributor
Posts: 337
Joined: Sun Dec 17, 2006 11:53 pm
Location: Sunshine Coast, Australia

Post by iknownothing »

maybe...

Code: Select all

$count = 1;
$cols = 3;

while ($sql_row = mysql_fetch_array($sql_result_types)) 
{
     echo "<td>" . $sql_row['name']. "</td>";

     if ($count == $cols){
          echo '</tr><tr>';
          $count = 0;
     }

     $count++;
}
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

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
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

So it is now:

Code: Select all

<table>
<tr>
	<?php
	$cols = 3;
	$i = 1;
	while ($sql_row = @mysql_fetch_array($sql_result_types))
	{
		echo "<td>".$sql_row['name']."</td>";
		if ($i == $cols)
		{
			echo "</tr><tr>";
			$i = 0;
		}
		$i++;
	}
	?>
</tr>
</table>
I'm not sure whether insufficient number of "<td>" elements at the end of the table could cause any problems to browsers' render.
Last edited by VladSun on Thu Aug 09, 2007 11:18 am, edited 1 time in total.
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

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
User avatar
iknownothing
Forum Contributor
Posts: 337
Joined: Sun Dec 17, 2006 11:53 pm
Location: Sunshine Coast, Australia

Post by iknownothing »

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.

And whats the purpose of the error suppressor?
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

1. Yes, I've noticed it - just forgot to edit it here.
2. Should I add aditional HTML code like that:

Code: Select all

<table>
<tr>
	<?php
	$cols = 3;
	$i = 1;
	while ($sql_row = @mysql_fetch_array($sql_result_types))
	{
		echo "<td>".$sql_row['name']."</td>";
		if ($i == $cols)
		{
			echo "</tr><tr>";
			$i = 0;
		}
		$i++;
	}
	while ($i <= $cols)
	{
		echo "<td></td>";
		$i++;
	}
	?>
</tr>
</table>
Last edited by VladSun on Thu Aug 09, 2007 11:15 am, edited 1 time in total.
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
iknownothing
Forum Contributor
Posts: 337
Joined: Sun Dec 17, 2006 11:53 pm
Location: Sunshine Coast, Australia

Post by iknownothing »

Yes, I suppose you could do that, but I'm not sure if it's necessary, but, best keep IE happy, if anything will have a whinge, its IE.

but you'll have to add:

Code: Select all

$i++
in the second while too.
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

:) I've just edited it ;)
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

I am more curious of using an array function (e.g. array_walk() ) to accomplish this. It should be related to matthijs's post.
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
TheMoose
Forum Contributor
Posts: 351
Joined: Tue May 23, 2006 10:42 am

Post by TheMoose »

VladSun wrote:I am more curious of using an array function (e.g. array_walk() ) to accomplish this. It should be related to matthijs's post.

Code: Select all

<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>
programmingjeff
Forum Commoner
Posts: 26
Joined: Fri Jan 05, 2007 10:56 am

Post by programmingjeff »

TheMoose wrote:
VladSun wrote:I am more curious of using an array function (e.g. array_walk() ) to accomplish this. It should be related to matthijs's post.

Code: Select all

<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.
Begby
Forum Regular
Posts: 575
Joined: Wed Dec 13, 2006 10:28 am

Post by Begby »

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.
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

Well, indeed the complexity increases to 0(2*n) - not O(n^2), but still there is an incremenet.
I really doubt you are going to notice any difference whatsoever by changing this to one loop.
Then ... what would happen if you have 10'000 users at the same time?!?
And it is still question of efficient programming ...
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Post by VladSun »

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
Post Reply