Page 1 of 2

Is it time to move away from procedural?

Posted: Thu Jul 26, 2007 8:36 am
by psurrena
This page pulls a specific project from the databse

Code: Select all

<?php
	# * WORK item * #
	
	$pid=$_GET['id'];
	require_once(DB);
	
	$query="SELECT P.project_id, P.project_name, P.project_des, P.category_id, P.location_id, C.category_full FROM project P, category C WHERE project_id='$pid' AND P.category_id=C.category_id";
	$result=mysql_query($query) OR DIE (mysql_error());
	$row=mysql_fetch_assoc($result);
	
	//Save for Later!
	$location=$row['location_id'];
	
	if (mysql_num_rows($result)){
		echo "<h3>".$row['project_name']."</h3>\n";
		echo "<p>".$row['project_des']."</p>\n";
		
		/*Check for Images */
		$path="images/work/".$row['project_id'].'/';
		$dir=opendir($path);
		while($name=readdir($dir)){
			if (preg_match("/t+([0-9]+\.jpg)$/",$name,$sp)){
				$nameb=ereg_replace('t','', $name);
				echo '<a href="'.BASE_URL.$path.$sp[1].'" rel="lightbox" title="'.$row['project_name'].'">';
				echo '<img class="work" src="'.BASE_URL.$path.$name.'" alt="'.$row['project_name'].'" width="140" />';
				echo "</a>&nbsp;\r";
			}
		}
		
		//PDF
		echo '<p><img src="'.BASE_URL.'images/pdf-icon.gif" /> <a class="workU" href="'.BASE_URL.'modules/pdf.php?pid='.$pid.'"> View Project Brief.</a></p>';
		
		//DEATAILS Section
		echo "<p><strong>Details</strong><br />\n";
		echo "Category: ";
		
		//Categories the project is listed under
		$a=explode(' ',$row['category_id']);
		foreach ($a as $key=>$value){
			$query="SELECT category_full, category_id FROM category where $value=category_id";
			$result=mysql_query($query) OR DIE (mysql_error());
			$row=mysql_fetch_assoc($result);
			$cat=$row['category_id'];
			if(mysql_num_rows($result)){
				echo '<a class="workU" href="'.BASE_URL.'category-'.$cat.'.html" class="underline">'.$row['category_full']."</a>\n";
			} else {
				echo "The project is not categorized.";
			}
		}
		echo "<br />\n";
		// Locaton of the project
		echo "Location: ";
		$query="SELECT location_id, location_ct, location_st, location_st_full, location_co FROM location WHERE location_id='$location'";
		$result=mysql_query($query) OR DIE (mysql_error());
		$row=mysql_fetch_assoc($result);
		$city=ereg_replace(' ','-', trim($row['location_ct']));
		$state=ereg_replace(' ','-', trim($row['location_st']));
		echo '<a class="workU" href="'.BASE_URL.'location-'.$city.'-'.$row['location_id'].'.html">'.$row['location_ct']."</a>";
		
		//IN PROGRESS
		if($row['location_st']){
			echo ', <a class="workU" href="'.BASE_URL.'location-'.$state.'-'.$row['location_id'].'.html">'.$row['location_st_full']."</a>\n";
		}
		
		if ($row['location_co'] != "USA"){
			echo ', <a class="workU" href="'.BASE_URL.'work.php?mode=loc&co='.$row['location_id'].'">'.$row['location_co'].'</a>';
		}
		echo "</p>\r";
		
		$query="SELECT project_id, people_fname, people_lname, people_mem, people_id FROM people WHERE project_id LIKE '%$pid%'";
		$result=mysql_query($query) or die (mysql_error());		
		if(!mysql_num_rows($result)){
			echo'';
		}else{
			echo "<p><strong>People</strong><br />";
			while($row=mysql_fetch_assoc($result)){
			$membership=ereg_replace(' ',', ', $row['people_mem']);	
				echo '<a class="workU" href="'.BASE_URL.'bio-'.$row['people_fname'].'-'.$row['people_lname'].'-'.$row['people_id'].'.html">'.$row['people_fname'].' '.$row['people_lname'].', '.$membership.'</a><br />';
			}
			echo "</p>\n";
		}

		echo '<p><strong><a class="work" href="'.BASE_URL.'category-'.$cat.'.html">< Project List</a></strong></p>';

	} else {
		echo 'There are no additional details available.';
	}
?>

Posted: Thu Jul 26, 2007 8:40 am
by Chris Corbyn
It doesn't strictly require that you move away from procedural. You certainly need to learn how to keep your presentation separate from your business logic though.

OOP helps in that respect, but it's not a requirement.

Posted: Thu Jul 26, 2007 8:42 am
by psurrena
What do yo mean by presentation separate from business logic?

Posted: Thu Jul 26, 2007 8:58 am
by sebastianrs
That simply means that you should separate your presentation elements like the HTML output from any behavioral elements--the code logic. Retrieve the information using a separate PHP file called something.inc.php or similar.

Posted: Thu Jul 26, 2007 9:32 am
by superdezign
Don't take that too literally, however. If something needs to be displayed, it needs to be displayed, but you shouldn't have any particularly long statements or any need for deep nested if-statements in your display file. Clean code means less clutter and more organization, and the cleaner your code, the easier it is to modify.

Posted: Thu Jul 26, 2007 9:48 am
by psurrena
So the code below really does not need to be displayed / could easily be an include. Is that what you're saying? Also, would this be a good piece of code to start learning oop with? I feel it's pretty straight forward.

Thanks for all the help!

Code: Select all

/*Check for Images */
$path="images/work/".$row['project_id'].'/';
$dir=opendir($path);
while($name=readdir($dir)){
		if (preg_match("/t+([0-9]+\.jpg)$/",$name,$sp)){
				$nameb=ereg_replace('t','', $name);
				echo '<a href="'.BASE_URL.$path.$sp[1].'" rel="lightbox" title="'.$row['project_name'].'">';
				echo '<img class="work" src="'.BASE_URL.$path.$name.'" alt="'.$row['project_name'].'" width="140" />';
				echo "</a>&nbsp;\r";
		}
}

Posted: Thu Jul 26, 2007 10:40 am
by superdezign
That would be an.. okay place to start. But then there's issues of what class(es) it would become. One for handling your images, one for handling your files, or one of each working together?

Posted: Thu Jul 26, 2007 10:57 am
by psurrena
Ideally it would be one of each working together.

Posted: Thu Jul 26, 2007 12:13 pm
by superdezign
psurrena wrote:Ideally it would be one of each working together.
Any idea how much control you'd give to either? The issue I was addressing was that either of them could be structured to be able to handle that entire function, so either you choose one, or somehow try to split it up.

Posted: Thu Jul 26, 2007 12:19 pm
by psurrena
If I understand correctly, almost total control. The only variable would be the path.

Posted: Thu Jul 26, 2007 12:24 pm
by superdezign
psurrena wrote:If I understand correctly, almost total control. The only variable would be the path.
I mean that particular function if broken up into being performed by two classes would need each class to perform certain parts of it. Which one would do more? Would the image class only define the image path and filename and the file class handle the rest, or would the file class only handle retrieving the file and the image class do the rest?

Posted: Thu Jul 26, 2007 12:31 pm
by psurrena
For my purposes now, naming of the file is specific as to control the order. So the images are 1.jpg, 1t.jpg, 2.jpg 2t.jpg, etc. What makes sense to me is, the file class would only to the retrieving and image would do the rest.

Posted: Thu Jul 26, 2007 12:46 pm
by superdezign
psurrena wrote:For my purposes now, naming of the file is specific as to control the order. So the images are 1.jpg, 1t.jpg, 2.jpg 2t.jpg, etc. What makes sense to me is, the file class would only to the retrieving and image would do the rest.
Makes sense. I'd get started on it. And try to leave some elbow room for later on, as you'll likely re-factor as you discover new things that you'll want to be able to do.

Posted: Thu Jul 26, 2007 12:49 pm
by psurrena
One last question, and I don't mean to drag this on. Why would I use two classes over one class with two functions?

Posted: Thu Jul 26, 2007 6:03 pm
by superdezign
psurrena wrote:One last question, and I don't mean to drag this on. Why would I use two classes over one class with two functions?
No one said you have to use two classes, you said you'd use two.

The whole purpose of classes is a separation of duties, so to speak. Every object is a worker on an assembly line to serve your data, and each of them has a task to perform and the section to handle.