[solved] Suggestion to clean this up?

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

User avatar
dallasx
Forum Contributor
Posts: 106
Joined: Thu Oct 20, 2005 4:55 pm
Location: California

[solved] Suggestion to clean this up?

Post by dallasx »

I'm not sure if this goes in here but I'll give it a whirl.

The following class function is to display an item(s). Is there a way I can clean this up? I honestly don't know where to begin. Yeah, it's hideous, I know.

Code: Select all

echo "<table width=\"700\"  border=\"0\" cellpadding=\"0\" cellspacing=\"1\">";
	echo "<form name=\"form".$fnumber."\" method=\"post\" action=\"add.php\">";
	echo "<tr>";
	echo "<td width=\"198\" rowspan=\"5\" align=\"center\" valign=\"middle\" bgcolor=\"#000066\" class=\"pic_box\"><img src=\"".$single_row[0]."\"></td>";
	echo "<td width=\"15\" rowspan=\"6\" valign=\"top\" bgcolor=\"#000066\" class=\"divider\">&nbsp;</td>";
	echo "<td width=\"369\" height=\"18\" valign=\"middle\" bgcolor=\"#CCCCCC\" class=\"title\">" . str_replace("&3nsquot","'",$row['item_name']) . "</td>";
	echo "<td width=\"110\" valign=\"middle\" bgcolor=\"#CCCCCC\"  class=\"title\">$" . number_format($number,2) . "</td>";
	echo "</tr>";
	echo "<tr>";
	echo "<td height=\"21\" colspan=\"2\" valign=\"top\" bgcolor=\"#CCCCCC\" class=\"info\">" . str_replace("&3nsquot","'",$row['color']) . " - Logo over " . $lp . ", " . $sl . ", " . str_replace("&3nsquot","'",$row['material']) . "</td>";
	echo "</tr>";
	echo "<tr>";
	echo "<td height=\"21\" colspan=\"2\" valign=\"top\" bgcolor=\"#CCCCCC\" class=\"info\">" . str_replace("&3nsquot","'",$row['gender']) . "</td>";
	echo "</tr>";
	echo "<td class=\"info\" height=\"21\" colspan=\"2\" valign=\"top\" bgcolor=\"#CCCCCC\">";
	echo "Sizes Available:<br>";
	if($gen == "Mens")
	{
		echo "Mens:";
		$sql_msizes = "SELECT m_product_size FROM product WHERE product_id='$prodid'";
		$result = mssql_query($sql_msizes);
		$mens_sizes = mssql_fetch_assoc($result);
		$ms = $mens_sizes['m_product_size'];
		$ms = unserialize($ms);
		echo "<select name=\"available_mens_sizes\">";
		echo "<option selected>Select a Size</option>";
		while (list($key, $value) = each($ms))
		{
			if($value > 0)
			{
				echo "<option value=\"".$key."\">".$key."</option>";
			}
		}
			echo "</select> Quantity: <input type=\"text\" size=\"4\" name=\"mens_size_quantity\"><br>";
	}
	if($gen == "Womens")
	{
		echo "Womens:";
		$sql_wsizes = "SELECT w_product_size FROM product WHERE product_id='$prodid'";
		$result1 = mssql_query($sql_wsizes);
		$womens_sizes = mssql_fetch_assoc($result1);
		$ws = $womens_sizes['w_product_size'];
		$ws = unserialize($ws);
		echo "<select name=\"available_womens_sizes\">";
		echo "<option selected>Select a Size</option>";
		while (list($key, $value) = each($ws))
		{
			if($value > 0)
			{
				echo "<option value=\"".$key."\">".$key."</option>";
			}
		}
		echo "</select> Quantity: <input type=\"text\" size=\"4\" name=\"womens_size_quantity\">";
	}
	if($gen == "Mens and Womens")
	{
		echo "Mens:";
		$sql_msizes = "SELECT m_product_size FROM product WHERE product_id='$prodid'";
		$result = mssql_query($sql_msizes);
		$mens_sizes = mssql_fetch_assoc($result);
		$ms = $mens_sizes['m_product_size'];
		$ms = unserialize($ms);
		echo "<select name=\"available_mens_sizes\">";
		echo "<option selected>Select a Size</option>";
		while (list($key, $value) = each($ms))
		{
			if($value > 0)
			{
				echo "<option value=\"".$key."\">".$key."</option>";
			}
		}
		echo "</select> Quantity: <input type=\"text\" size=\"4\" name=\"mens_size_quantity\"><br>";
		echo "Womens:";
		$sql_wsizes = "SELECT w_product_size FROM product WHERE product_id='$prodid'";
		$result1 = mssql_query($sql_wsizes);
		$womens_sizes = mssql_fetch_assoc($result1);
		$ws = $womens_sizes['w_product_size'];
		$ws = unserialize($ws);
		echo "<select name=\"available_womens_sizes\">";
		echo "<option selected>Select a Size</option>";
		while (list($key, $value) = each($ws))
		{
			if($value > 0)
			{
				echo "<option value=\"".$key."\">".$key."</option>";
			}
		}
		echo "</select> Quantity: <input type=\"text\" size=\"4\" name=\"womens_size_quantity\">";
	}
	echo "</td>";
        echo "  <tr>";
        echo "    <td class=\"info\" colspan=\"2\" rowspan=\"2\" valign=\"top\" bgcolor=\"#CCCCCC\"<span class=\"style4\">Description:<br>" . str_replace("&3nsquot","'",$row['item_description']) . "</td>";
        echo "  </tr>";
        echo "  <tr>";
        echo "<td bgcolor=\"#000066\" class=\"info\" height=\"29\" valign=\"middle\" align=\"center\">
         		<input name=\"item_id\" type=\"hidden\" value=\"$prodid\">
            		<input name=\"price\" type=\"hidden\" value=\"$number\">
            		<input type=\"submit\" name=\"Submit\" value=\"Add to Cart\">
            	  </td>";
        echo "  </tr>";
        echo "</table>";
	echo "<p></p>";
	$fnumber++;
       	mssql_close($link);
        echo "</body>";
        echo "</html>";
Last edited by dallasx on Thu Nov 10, 2005 3:16 pm, edited 1 time in total.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Whitespace is your friend!

Separate blocks, particularly between if's and other lines.

Instead of multiple lines of echo's, use something similar to the following:

Code: Select all

echo
"<html>
    <head>
        <title>boo</title>
    </head>
    <body>
        Boo too!
    </body>
</html>\n";
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

I'd suggest switching to templates :)
User avatar
dallasx
Forum Contributor
Posts: 106
Joined: Thu Oct 20, 2005 4:55 pm
Location: California

Post by dallasx »

Jenk wrote:Whitespace is your friend!

Separate blocks, particularly between if's and other lines.

Instead of multiple lines of echo's, use something similar to the following:

Code: Select all

echo
"<html>
    <head>
        <title>boo</title>
    </head>
    <body>
        Boo too!
    </body>
</html>\n";
You know what, I used to do that. Then I relied heavily on conditionals so ended up breaking up every other line. After while, I got used too used to doing that. But yeah, it makes sense :) I'll do that. Thanks!
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Perhaps it's the Confederate flag that is causing your backward thinking.

Templates with blocks should solve your code problem.
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Post by Charles256 »

also, instead of escpaing your " why not just use ' ?
User avatar
n00b Saibot
DevNet Resident
Posts: 1452
Joined: Fri Dec 24, 2004 2:59 am
Location: Lucknow, UP, India
Contact:

Post by n00b Saibot »

or better still use heredoc operator - <<< 8)

Code: Select all

echo <<<END
<html> 
    <head> 
        <title>boo</title> 
    </head> 
    <body> 
        Boo too! 
        $SomeFunkyVar
    </body> 
</html>
END;
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Post by Charles256 »

now that's damn nifty..
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

Move styling to css.
McGruff
DevNet Master
Posts: 2893
Joined: Thu Jan 30, 2003 8:26 pm
Location: Glasgow, Scotland

Post by McGruff »

Normally code is separated into three main layers: presentation, domain and data access.

Presentation is the UI. The presentation layer receives input, decides what to do with it, and creates a view.

The domain layer is your business logic. Domain objects would take raw data and add some kind of value to it for example, calculating the interest due on an account. Sometimes there is no domain logic to do; you simply grab some data and display it.

The data access layer is the CRUD code which reads/writes to persistent storage (usually a database).
User avatar
dallasx
Forum Contributor
Posts: 106
Joined: Thu Oct 20, 2005 4:55 pm
Location: California

Post by dallasx »

arborint wrote:Perhaps it's the Confederate flag that is causing your backward thinking.

Templates with blocks should solve your code problem.
Hehehe.

I'll tell you exactly what it is... I suffer from ADD when it comes to this stuff. I have all the ideas and I want to get them all done at the same time. Impossible. When I teach myself, my thought structure is nowhere near focused. I'm always thinking of what I can do next. That's why I ask so many questions :)

I'm trying to focus on one thing at a time.

On that note, I really appreciate everyone's help with everything, seriously. It's nice to be able to come to a board that has professionalism and not a bunch of rowdy kids cussin'.
User avatar
jurriemcflurrie
Forum Commoner
Posts: 61
Joined: Wed Jul 06, 2005 7:14 am
Location: Den Haag, the Netherlands

Post by jurriemcflurrie »

feyd / arborint:
What do you mean by templates axactly?
User avatar
n00b Saibot
DevNet Resident
Posts: 1452
Joined: Fri Dec 24, 2004 2:59 am
Location: Lucknow, UP, India
Contact:

Post by n00b Saibot »

Templates are like set of skeleton in which you plugin your custom fillings and have a completed body ready :) e.g. a simple page template

Code: Select all

<html>
<head>
<title>{TITLE}</title>
</head>
<body>
<h1>{PAGE TITLE}</h1>
<table cellpadding="5" cellspacing="5" border="0">
<tr>
<td>{CONTENT}</td>
</tr>
</table>
</body>
</html>
and prcoessing it is simple

Code: Select all

<?
//Set Template Vars...
$Title = 'Jean Claude Van Dame';
$PgTitle = 'Jean Claude Van Dame';
$Content = 'I am a huge fan of Van Dame... What action!!!';

//Fetch Template Contents...
$Page = file_get_contents('template.html');

//Fit in our custom Contents...
$Page = str_replace(array('{TITLE}', '{PAGE TITLE}', '{CONTENT}'), array($Title, $PgTitle, $Content), $Page);

//display it... (finally  )
echo $Page;
?>
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post by mickd »

this site has a good tutorial on a template system

http://codewalkers.com/tutorials/58/1.html

on page 2, this is how he displays a page using OOP

Code: Select all

<?php
require_once("lib/template.php");

$page = new Page("template.html");

$page->replace_tags(array(
  "title" => "HOME",
  "descript" => "Welcome to my website!",
  "main" => "dat/index.dat",
  "menu" => "dat/menu.dat",
  "left" => "dat/submenu.dat",
  "right" => "dat/right.dat",
  "footer" => "dat/footer.php"
));

$page->output();
?>
the tutorial is 9 pages (extremely short pages) and its a good read for people that dont know how it works.
User avatar
dallasx
Forum Contributor
Posts: 106
Joined: Thu Oct 20, 2005 4:55 pm
Location: California

Post by dallasx »

Thanks for the input guys!
Post Reply