Dynamic homepage with PHP - making things too complicated?

Question

I am trying to show the 6 most popular products on my homepage from left to right, in two rows. My code actually works... but I have the feeling I am making things way too complicated. Is there a simpler way to do this?

Here is my code:

$dynamicList = "";
$sql = mysql_query("SELECT * FROM products ORDER BY popularity DESC LIMIT 6");
$productCount = mysql_num_rows($sql); //Count the output amount
if($productCount>0){
 $currentRow=0;
 $dynamicListLeft='<div id="homepageLeftProduct">';
 $dynamicListMiddle='<div id="homepageMiddleProduct">';
 $dynamicListRight='<div id="homepageRightProduct">';
 while($row=mysql_fetch_array($sql)){
 $currentRow=$currentRow+1;
 $id = $row["id"];
 $product_name = $row["product_name"];
 $date_added=strftime("%b %d, %Y", strtotime($row["date_added"]));
 if($currentRow==1 || $currentRow==4) {
 $dynamicListLeft .='<p>
 <a href="product.php?blablabla"><img class="homepagePic" src="inventory_images/4.jpg" /></a>
 </p><br/>';
 } else {
 if($currentRow==2 || $currentRow==5) {
 $dynamicListMiddle .='<p>
 <a href="product.php?blablabla"><img class="homepagePic" src="inventory_images/4.jpg" /></a>
 </p><br/>';
 } else {
 if($currentRow==3 || $currentRow==6){
 $dynamicListRight .='<p>
 <a href="product.php?blablabla"><img class="homepagePic" src="inventory_images/4.jpg" /></a>
 </p><br/>';
 }
 }
 }
 }
 $dynamicListLeft .='</div>';
 $dynamicListMiddle .='</div>'; 
 $dynamicListRight .='</div>';
 $dynamicList .=$dynamicListLeft;
 $dynamicList .=$dynamicListMiddle;
 $dynamicList .=$dynamicListRight;
}

Later on in my body I echo the dynamiclist.

I hope somebody can show me the way to make this smoother?

Thanks!

Answer 1

Let's go for a step-by-step cleanup.

1) Get rid of repetition by using a variable to store your html code common to the different branches.

2) Remove useless levels of indentation to make things easier to follow.

The inside of the while loop is now :

 $currentRow=$currentRow+1;
 $id = $row["id"];
 $product_name = $row["product_name"];
 $date_added=strftime("%b %d, %Y", strtotime($row["date_added"]));
 $link = '<p><a href="product.php?blablabla"><img class="homepagePic" src="inventory_images/4.jpg" /></a></p><br/>';
 if($currentRow==1 || $currentRow==4) {
 $dynamicListLeft .= $link;
 } else if($currentRow==2 || $currentRow==5) {
 $dynamicListMiddle .= $link;
 } else if($currentRow==3 || $currentRow==6){
 $dynamicListRight .= $link';
 }

(I could move the definition of link out of the loop but I guess in your real situation, the content depends on $row).

3) You can try to store your content in an array instead of using different variables. It removes a bit of logic and makes things easier to update if you want 2 or 4 columns in the future. It also removes some code duplication :

$cols=array('','','');
$nbCol=count($cols);
$currentRow=0;
while($row=mysql_fetch_array($sql)){
 $id = $row["id"];
 $product_name = $row["product_name"];
 $date_added=strftime("%b %d, %Y", strtotime($row["date_added"]));
 $link = '<p><a href="product.php?blablabla"><img class="homepagePic" src="inventory_images/4.jpg" /></a></p><br/>';
 $cols[$currentRow%$nbCol] .= $link;
 $currentRow=$currentRow+1;
}
for ($col as $i => $content)
{
 $dynamicList .= '<div id="homepageProductCol' . $i . '">' . $content . '</div>';
}

4) So far, the comments were about the php code and not the HTML output it produces. My last comment is more of a question but wouldn't there be a way to have your HTML defined in such a way that you don't need such a complicated logic. Basically, at the moment, the whole point is that your HTML code contains reference to your item in this order : 1,4,2,5,3,6. Things would be easier if you could have them in the natural order (1,2,3,4,5,6) in your HTML code and then update your HTML to display them in such a such a way. My HTML skills are way too limited to answer this question...