PHP best practices?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
Zander1983
Forum Newbie
Posts: 20
Joined: Mon Mar 21, 2011 2:26 pm

PHP best practices?

Post by Zander1983 »

Hi
I've developed a website myself over the last 6 months. I've gotten some investment to develop it further and will now have 2 develoeprs working with me. Its a market place style website so high hits are expected and speed is a must. I was wondering, is my code up to scratch? what exactly are best practices in php? Here's an example of how i get all products on the index page:

Code: Select all

			$sql = "SELECT si.Name, si.ShopItemID, si.Active, si.InStock, si.DateAdded, si.Price, s.CountyID, si.Url as ItemUrl, s.Url as ShopUrl, si.Approved, ii.Url, ii.Active, ii.Front, ii.Thumb, ii.OrderImage, s.CategoryID, s.ShopID, s.Active ";
			$sql = $sql."FROM shopitem si ";  
			$sql = $sql."Inner Join shop s On (s.ShopID = si.ShopID) ";
			$sql = $sql."left Join itemimage ii on (ii.ShopItemID = si.ShopItemID And ii.OrderImage = 1) Where 1=1 and si.Active = 1 and si.InStock > 0 and s.Active = 1 and si.Approved = 1 ";
			if(!$categoryId==0 and !$categoryId==""){
				$sql = $sql."And CategoryID = ".mysql_real_escape_string($categoryId);	
			}
			$sql .= " Group By si.ShopItemID ";
and to display it:

Code: Select all

			 while ($row = mysql_fetch_array($result)) {
				echo "<div class='shopItems'>";
				echo "<div id='itemIndex'>".$x.".</div>";
					if(!$row['Url']==NULL){
						echo "<div id='centered'><a href='".WEBSITE_DOMAIN."shops/".$row['ShopUrl']."/".$row['ItemUrl']."'><img alt='".htmlspecialchars($row['Name'])."' title='".htmlspecialchars($row['Name'])."' height='135' width='170' src='images/".htmlspecialchars($row['Front'])."'/></a></div>";
					}
					else{
						echo "<div id='centered'><a href='".WEBSITE_DOMAIN."shops/".$row['ShopUrl']."/".$row['ItemUrl']."'><img alt='".htmlspecialchars($row['Name'])."' title='".htmlspecialchars($row['Name'])."' height='135' width='170' src='images/no-img.jpg'/></a></div>";
					}
					echo "<div id='centered'><a href='".WEBSITE_DOMAIN."shops/".$row['ShopUrl']."/".$row['ItemUrl']."'>".htmlspecialchars($row['Name'])."</a></div>";
					echo "<p id='centered'>€".htmlspecialchars($row['Price'])."</p>";
				echo "</div>";
				
				If($x%4==0){
				echo "<div id='clear'></div>";
				echo "<div id='divider'></div>";
				
				}
				$x++;
			 }
Is there anything wrong with this approach? Is it best practice and/or faster to use object oriented programming? So I would have a ShopItems class, with members which reflect the attributes of the ShopItems table, and call a function of it to run the sql and set the vales of the members. Am I going in the right direction, towards better practices? Or is what I have done fine? I have used the top approach throught the website, but worry that its a bit of an old-fashioned approach...any advice?

thanks
Mark
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: PHP best practices?

Post by Christopher »

You don't show where you get $categoryId. It should be validated and filtered so attempted SQL injection does not create a SQL error.
(#10850)
Post Reply