Code Improvement

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
freakunleash
Forum Newbie
Posts: 1
Joined: Tue Jan 24, 2012 10:30 am

Code Improvement

Post by freakunleash »

Hi All,

I'm new to PHP and I'm working on creating a personal movie database using PHP & MYSQL. As of now it is working fine as per my expectation but I I'm using lots of repeated code to insert or select data from database. Below are 3 codes which are being used to query multiple tables to either insert or select data. I just need to replace the "cast" word with "producer" or "director" or "writer" and I can use the same query.

What is the better way improve the code so that I can reduce the no. of lines for the code.

1st Code is for insert in the table.

Code: Select all

<?php

			
//******************************************* CAST UPDATE *******************************************//

$sql = array(); 
    foreach( $cast as $row ) {
	$imdb = mysql_real_escape_string($row['imdb']);
	$name = mysql_real_escape_string($row['name']);
	
       $sql[] = '("'.$imdb.'", "'.($name).'" )';
    }
    $result = 'INSERT INTO cast (imdb, name) VALUES '.implode(',', $sql).' ON DUPLICATE KEY UPDATE lastupdate = NOW()';
	mysql_query($result) or die('log error with' .mysql_error());

//******************************************* TITLE_CAST UPDATE *******************************************//	

	$sql_query = "INSERT INTO title_cast (id_title, id_cast) 
					SELECT $title_id, hi.cast_id
					FROM cast_inserts hi
					WHERE hi.conn_id = CONNECTION_ID()";
	if (!mysql_query($sql_query,$con))
	  {
	  die('Error: ' . mysql_error());
	  }
		$sql_del = "DELETE FROM cast_inserts WHERE conn_id = CONNECTION_ID()";
	if (!mysql_query($sql_del,$con))
	  {
	  die('Error: ' . mysql_error());
	  }

?>
2nd code for select query

Code: Select all

<?php

$sql_cast = "SELECT *
			FROM
			  title_cast
			  INNER JOIN title ON (title_cast.id_title = title.id)
			  INNER JOIN `cast` ON (title_cast.id_cast = `cast`.id)
			WHERE
			  title_cast.id_title = '$movieid'";
$result_cast = mysql_query($sql_cast) or die('log error with' .mysql_error());

?>
          <div class="desc">CAST</div>
            
            <?php while ($row = mysql_fetch_array($result_cast)) {?>
            <?php 
				$id = $row['imdb'];
				$name = $row['name'];
				$img = $row['photo_localurl'];
				$poster = str_replace("./", "lib/", $img);
//		$poster_cast = str_replace("../", "", $img);
			?>
            <div class="details1"><table><tr>
            	<td class="personimg"><img src="<?php if(!empty($poster)){echo $poster;}else{echo $dft_img;}?>" width="32" height="42" alt="<?php echo $name;?>" ></td>
              	<?php echo "<td class='personname'><a href='persons.php?det=cast&perid=".$id."'> ".$name." </a></td>";?>
              	<td class="personrole">"personrole"</td></tr></table>        
            </div>
            <?php } ?>
3rd query again for select

Code: Select all

<?php
	$qAsCast = "SELECT 
			  title.id,
			  title.title,
			  title.`year`,
			  title.poster,
			  title.imdb,
			  title.ratings
			FROM
			  title
			  INNER JOIN title_cast ON (title.id = title_cast.id_title)
			  INNER JOIN `cast` ON (title_cast.id_cast = `cast`.id)
			WHERE
			  `cast`.imdb = $imdb";
$rAsCast = mysql_query($qAsCast);
	
	
	echo "<div class='sidecont'>";
			
				$total = mysql_num_rows($rAsCast);
					if($total == '0' ){ echo "$msg";}
					else{
					while ($row = mysql_fetch_array($rAsCast)) {
					
						$id = $row['id'];
						$title = $row['title'];
						$year = $row['year'];
						$poster = $row['poster'];
						$poster = str_replace("./", "lib/", $poster);
						$rating = $row['ratings'];
						echo "<table><tr><td>";
						echo "<div id='a'>";
						echo "<div id='b'>".$title.'</div>';
						echo "<div id='c'>".$year.'</div>';
						echo "<div id='f'>".rat($rating).$rating.'</div>';
						echo "<div id='d'><a href='movies.php?movieid=$id'><img src='$poster' alt='' border='1' align='center' width='107' height='157' /></a></div>";
						echo "</div>";
						echo "</td></tr></table>";
						}
					}
	echo "<div>" 
?>
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Code Improvement

Post by social_experiment »

A good place to start is looking at functions; functions make it easier to re-use code
http://php.about.com/od/learnphp/ss/php_functions_4.htm
A primer on the subject. ^
freakunleash wrote:I just need to replace the "cast" word with "producer" or "director" or "writer" and I can use the same query.
If all your database tables are the same then yes this would be possible. The thing about writing 'good' functions is that you have to make them as re-useable as possible; instead of changing a keyword, why not modify the function to accept a query? This way the function isn't limited to just inserting or just updating, you could use it anywhere a mysql query is required.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
Post Reply