Jump to content

Is there a reason for NOT having a curly brace after if's or else's?


YourAdrenalineFix
 Share

Recommended Posts

Hi, I'm a fairlly Novice webmaster and recently I've been trying to learn all there is to learn with PHP (or close to it ;) ) Anyways... Now I know just enough to be dangerous (ie: I can crash a page I'm working on lol) and now I'm trying to customize a classifieds script I have running on my site (as well as fix a bunch of stuff that either looks messy or... let's just say something I wouldn't ever use) but I keep finding pages with no { after else. The reason I write is because I'm now on a crusade to correct all of this on every page but doing so is crashing pages and requiring HOURS of trying to figure out WTH?? I'm not really interested in any other classifieds scripts, Infact I feel blessed to have the time to learn all of this and be able to see unlimited potential. Anyways (again) I keep chasing down empty else's and if's and doing my best to correct them but as you can imagine when I place a { after an if or else, it often crashes the page and I'm wondering if there is a reason for an absence of a { ?? Bad Coding?? For example: (Since you may overlook my Thanks a mile below, I'll tell you now. I Thank You All In Advance!!) BTW, This is a file BEFORE I've touched anything.

<?require_once('z-mysql_ini.php');$mysql = new mysql();  //Browse$node = new sqlNode();$node->table = "categories";$node->select = "*";$node->where = "where ID = ".intval($_REQUEST['fatherID']);  if(($categoryR = $mysql->select($node)) === false )  die($mysql->debugPrint()); if(mysql_num_rows($categoryR)<1){  die("<script>window.location='/';</script>");}$category = mysql_fetch_assoc($categoryR);  //Make $PageTitle the title which a user enters for their ad followed by on your adrenaline fixs free classifieds$PageTitle = $category['PageTitle'];$metadescription = $category['MetaDescription'];$metakeywords = $category['MetaKeywords']; include('header.php'); ?> <!-- Begin Browse.php --><table align="center" width="580px"><tr><td valign="top"> <? echo "<a href='http://classifieds.your-adrenaline-fix.com/'>Classifieds Home</a>";echo breadCrumb(intval($_REQUEST['fatherID']),$mysql);	echo " » <a href='add.php?fatherID=".intval($_REQUEST['fatherID'])."'>List Your Ad for Free</a>"; echo $category['PageHeader'];  if( ($category['TypeID'] != "") && ($category['Locked'] == 'yes') ) {  //Fetch ad type information  $node = new sqlNode();  $node->table = "types";  $node->select = "*";  $node->where = "where ID = ".intval($category['TypeID']);   if(($typeR = $mysql->select($node)) === false )   die($mysql->debugPrint());    if(mysql_num_rows($typeR)<1){   die("<script>window.location='/';</script>");  }  $type = mysql_fetch_assoc($typeR);   //Fetch all listings for category order by date posted asc  $node = new sqlNode();  $node->table = sprintf("`tt_%s`", abs(intval($category['TypeID'])));  $node->select = "*, (`ExpireDate` < NOW()) as `Expired`";  $node->where = "where CategoryID = ".intval($category['ID']);   if(!$superUser){   $node->where .= " and (`ExpireDate` > NOW())";  }   //If sort by  if( ($_GET['SortCol'] != "") && ($_GET['SortBy'] != "") )   $node->orderby = sprintf("order by `%s` %s, `Featured` desc, `PostDate` desc", mysql_real_escape_string($_GET['SortCol'], $mysql->conn), mysql_real_escape_string($_GET['SortBy'], $mysql->conn));  else   $node->orderby = "order by `Featured` desc, `PostDate` desc";	   //echo $mysql->selectSql($node);   if(($result = $mysql->select($node)) === false )   die($mysql->debugPrint());   $num_of_listings = mysql_num_rows($result);   $results_per_page = 10;  $pageNum = $_GET['pageNum'];  if($pageNum == "")   $pageNum = 0;    $limit = " LIMIT ".($pageNum*$results_per_page)." , ".($results_per_page)." ";  $node->orderby .= $limit; 	  //DEBUG  //echo $mysql->selectSql($node);   //Get page results  if( ($result = $mysql->select($node)) === false )   die($mysql->debugPrint());    //Fetch browse columns -- user defined  $node = new sqlNode();  $node->table = "customfields";  $node->select = "*";  $node->where = "where TypeID = ".intval($category['TypeID'])." and BrowsePage = 'yes'";  $node->orderby = "group by GroupID order by `Type` asc";//  $node->orderby = "group by GroupID order by `Order` asc";    if(($colRS = $mysql->select($node)) === false )   die($mysql->debugPrint());   //Story columns into array  $column = array();  while($row = mysql_fetch_assoc($colRS))   array_push($column,array($row['Title'],$row['Type'],$row['IsLink'],$row['LinkText'],$row['Target']));   echo "<h4>$num_of_listings ".$category['Title']." Listed</h4>";   if($num_of_listings == 0){   echo $category['PageFooter'];   echo "</td>";   echo "</tr>";   echo "</table>";    include('footer.php');     die();  }   //echo show page links  if($num_of_listings > $results_per_page){   //Get total pages   $pageCount = ($num_of_listings / $results_per_page);     for($i=0; $i < $pageCount; $i++){	if($i == $pageNum)	 $html[] = "<strong>".($i+1)."</strong>";	else	 $html[] = "<a href='browse.php?fatherID=".$_GET['fatherID']."&pageNum=$i'>".($i+1)."</a>";   }     echo "Page: ";   echo @implode(' | ',$html);   if( $pageNum < ($pageCount-1) )	echo " | <a href='browse.php?fatherID=".$_GET['fatherID']."&pageNum=".($pageNum+1)."'>Next Page</a>";   }   echo "<a name='browse.php'></a>";  //Print sort form  echo "<form action='browse.php' method='get'>";  echo "<p>Sort By: ";  echo "<select name='SortCol'>";  if($superUser){   echo "<option value='ExpireDate'>Expire Date</option>";  }  foreach($column as $value){   echo "<option value='$value[0]' ";   if($_GET['SortCol'] == $value[0])	echo "selected";   echo ">$value[0]</option>";  }  echo "</select>";   echo "<select name='SortBy'>";  echo "<option value='asc' ";  if($_GET['SortBy'] == 'asc')   echo "selected";  echo ">Asc</option>";  echo "<option value='desc' ";  if($_GET['SortBy'] == 'desc')   echo "selected";  echo ">Desc</option>";  echo "</select>";  foreach($_GET as $name => $value){   if( ($name != "SortBy") && ($name != "SortCol") )	echo "<input type='hidden' name='$name' value='$value'>";  }  echo "<input type='submit' value='Go'>";  echo "</form>";										  echo "<table cellpadding=2 width='100%'>";  //For each listing found  while($ad = mysql_fetch_assoc($result)){   echo "<tr>";   echo "<td colspan=2 class='subHeader'>";   echo "<table border=0 cellspacing=0 cellpadding=0 width='100%'>";   echo "<tr>";   echo "<td width='70%'>";   echo "<strong>".$ad['Title']."</strong>";   if($ad['Featured'] == 'yes'){	echo " <font color='green'><strong>*Featured</strong></font>";   }     echo "</td>";   echo "<td align='right'>";   echo "<strong>ID: ".$type['ID']."-".$ad['ID']."</strong>";   echo "</td>";   echo "</tr>";   echo "</table>";   echo "</td>";   echo "</tr>";	   if($ad['Featured'] == 'yes'){	//Fetch Featured settings	$node = new sqlNode();	$node->table = "featurepackage";	$node->select = "*";	$node->where = "where ID = 1";  	if(($fpackage = $mysql->select_assoc($node)) === false )	 die($mysql->debugPrint());  	if($fpackage['Highlight'] == 'yes')	 echo "<tr bgcolor='".$fpackage['HighlightColor']."' >";	else	 echo "<tr>";   }   else	echo "<tr>";     if($type['Photos'] == 'yes'){	//Fetch small photo for ad	$node = new sqlNode();	$node->table = "photos";	$node->select = "*";	$node->where = "where TypeID = ".intval($type['ID'])." and ListingID = ".intval($ad['ID']);	$node->orderby = "Order By `POrder` ASC LIMIT 1";  	if(($photoRS = $mysql->select($node)) === false )	 die($mysql->debugPrint());		echo "<td width='150px' valign=top  >";	  	if(mysql_num_rows($photoRS) > 0){	 $photo = mysql_fetch_assoc($photoRS);	 echo "<a href='detail.php?fatherID=".$_REQUEST['fatherID']."&TypeID=".intval($type['ID'])."&ListingID=".intval($ad['ID'])."'>";	 echo "<img border=0 vspace=4 src='admin/photos/uploads/small_thumbs/tn_".$photo['Location']."' alt='Thumbnail Image Representing Item Listed'></a>";	}	else{   echo "<img src='image-files/tn_noimage.jpg' vspace=4 class='searchImage' alt='Thumbnail to Show No Image Available'></a>";	}  		    	echo "</td>";   }   echo "<td valign=top align=left>";     $num_of_columns = @count($column);   $count = 0;   $break = ceil($num_of_columns/2);     //If no text area make columns into two   foreach($column as $value){	if($value[1] == "textarea")	 $textarea = true;   }   $width= "width='25%'";   if(!$textarea){	echo "<table border=0>";	echo "<tr>";	echo "<td width='50%' valign=top>";	$width= "";   }     echo "<table class='innerSearchTable' width='100%'>";   if($ad['Expired']){	echo "<tr>";	echo "<td colspan=2 $width><font color='red'>Expired on ".$ad['ExpireDate']."</font></td>";	echo "</tr>";   }   foreach($column as $value){	echo "<tr>";	if($value[1] != "textarea"){	 echo "<td $width><strong>$value[0]</strong></td>";	 echo "<td>";	}	else{	 echo "<td colspan='2'>";	 echo "<strong>$value[0]</strong><br>";	}	if(!is_array($ad[$value[0]])){	 //If is HTML link	 if($value[2] == 'yes'){	  echo "<a href='".$ad[$value[0]]."' target='$value[4]'>$value[3]</a>";	 }	 else	  echo $ad[$value[0]];	 echo "</td>";	}	else{//Else is array -> print array value	 print_r($ad[$value[0]]);	}		echo "</tr>";	if($break == $count){	 echo "</table>";	 if(!$textarea){	  echo "</td>";	  echo "<td width='50%' valign=top>";	 }	 echo "<table class='innerSearchTable' width='100%'>";	}	$count++;   }   echo "<tr><td colspan=2>";   echo "<a href='detail.php?fatherID=".$_REQUEST['fatherID']."&TypeID=".$category['TypeID']."&ListingID=".$ad['ID']."'>View Details</a>,<br>";   echo "<a href='save.php?fatherID=".$_REQUEST['fatherID']."&TypeID=".$category['TypeID']."&ListingID=".$ad['ID']."'>Save Listing</a> <br>";  		     if($superUser){	echo "<a style='color:green' href='edit.php?TypeID=".$category['TypeID']."&ListingID=".$ad['ID']."'>Edit</a> | ";	echo "<a style='color:green' href='javascript: if(confirm(\"Do you want to delete?\")) window.location= \"delete.php?TypeID=".$category['TypeID']."&ListingID=".$ad['ID']."\"'>Delete</a>";   }   echo "</td></tr>";     echo "</table>";     if(!$textarea){	echo "</td>";	echo "</tr>";	echo "</table>";   }     echo "</td>";   echo "</tr>";  }  echo "</table>";}elseif($category['Locked'] == 'no'){  $node = new sqlNode();  $node->table = "categories";  $node->select = "*";  $node->where = "where FatherID = ".intval($category['ID']);  $node->orderby = "order by Title asc";   if(($result = $mysql->select($node))===false)   die($mysql->debugPrint());    $total_subcats = mysql_num_rows($result);  $break = ceil($total_subcats/2);  $count = 0;   echo "<table border=0 >";  echo "<tr>";  echo "<td valign=top>";  while($subcat = mysql_fetch_assoc($result)){   echo "<table>";   echo "<tr>";   echo "<td>";   echo "<a href='browse.php?fatherID=".$subcat['ID']."' title='".$subcat['Title']."'>";   echo "<img src='icons_folder/folder.jpg' border=0 alt='".$subcat['Title']."' >";   echo "</a>";   echo "</td>";   echo "<td>";   echo "<div class='subCat'>";   echo "<a href='browse.php?fatherID=".$subcat['ID']."'>";     echo $subcat['Title']."</a>";   if($SitesAdministrativeSettings['ShowNumAds'] == 'yes'){	$number_of_ads = fetchNumAds(intval($subcat['ID']),$mysql) + fetchSubCats(intval($subcat['ID']),$mysql);	if( ($number_of_ads > 0) || ($SitesAdministrativeSettings['ShowGTZero'] == 'no') )	 echo " (" . $number_of_ads . ")";     }   echo "</div>";   echo "</td>";   echo "</tr>";   echo "</table>";   if($count == $break){	echo "</td><td valign=top>";   }   $count++;  }   echo "</tr>";  echo "</table>";} echo $category['PageFooter'];echo "</td>";   echo "</tr>";   echo "</table>"; include('footer.php'); ?>

Edited by YourAdrenalineFix
Link to comment
Share on other sites

It's odd that you refer to a { without referring to both { and }, since they always come in pairs. Do you think you're crashing because you are mistakenly adding one brace instead of wrapping a set of statements in a pair of braces? Anyway It is technically correct to execute a single statement after a conditional (if, while, and so on) without enclosing it in braces. But it is generally not good practice. The reason is that you often end up editing your code so that multiple statements follow a conditional. In that case, you need to add the braces. And sometimes people forget. If the braces had been there from the start, adding more statements is very easy.

Edited by Deirdre's Dad
Link to comment
Share on other sites

Hi Deidre's Dad, Thanks so much for helping me with some of this mind boggling code. If you wouldn't mind, Please advise; Am I smart to go thru each page and add { and } where they are not?(sorry I didn't mention the } in my 1st post but I "meant it" and I presumed you could read my mind) For example, In this post I'm providing a cleaned up version of code (which is a different page than above but took me a full day to get to NOT crash so I HAVE spent some time trying to learn all of this.) and if you wouldn't mind, Could you please look over this code and advise if all looks correct? (and If it would be smart to continue like this with pages that look a mess (like first posts code)) Thanks so much once again!!

<?phpinclude('header.php');echo "<table class='IECenterContent'>";   echo "<tr>";   echo "<td width='100%'>";   echo breadCrumb(intval($_REQUEST['fatherID']),$mysql);  if(!$login) {  echo "<h2>Please login to create an ad.</h2>";  include ('z-login-form.php');  echo "</td>";  echo "</tr>";  echo "</table>";   include('no-ad-footer.php');} else {   $node = new sqlNode();   $node->table = "categories";   $node->select = "*";   $node->where = sprintf("WHERE `ID` = %s", intval($_GET['fatherID']));   $result = $mysql->select($node) or die("<script>window.location='/';</script>");    if(mysql_num_rows($result)<1) {	 die('No Categories Exist with that FatherID');    }    $category = mysql_fetch_assoc($result);   //$category = Honda Dikes locked   if($category['Locked'] == 'no') {   $node = new sqlNode();   $node->table = "categories";   $node->select = "*";   $node->where = "WHERE `FatherID` = ".intval($category['ID']);    if(($result = $mysql->select($node)) === false) {	 die();    }      if(mysql_num_rows($result) > 0) {     echo "<form action='add.php' method='GET'>";    echo "Please select subcategory of ";    echo "<strong>".$category['Title']."</strong>";    echo "<select name='fatherID'>";    while($subcat = mysql_fetch_assoc($result)) {	 echo "<option value='".$subcat['ID']."'>";	 echo $subcat['Title'];    }    echo "</option>";    echo "</select>";    echo '<input type="submit" value="Get Started Listing My Ad">';    echo "</form>";    }     } else { echo "<div class='error'>This category is currently locked.</div>"; }    if($SitesAdministrativeSettings['FreeMode'] == 'no') {   $node = new sqlNode();   $node->table = "accounting";   $node->select = "*";   $node->where = "WHERE `MemberID` = ".intval($_SESSION['memberID'])." AND `CategoryID` = ".intval($category['ID'])." AND `Used` = 'no'";    if(($result = $mysql->select($node)) === false) {	 die();    }       if(mysql_num_rows($result) <= 0) {    echo "<script>window.location='packages.php?fatherID=".$category['ID']."'</script>";    echo "<a href='packages.php?fatherID=".$category['ID']."'>Please Select Package to Continue</a>";			 echo "</td>";    echo "</tr>";    echo "</table>";     include('no-ad-footer.php');    die();    }    $package = mysql_fetch_assoc($result);    }	   $node = new sqlNode();  $node->table = "customfields";  $node->select = "*";  $node->where = "WHERE `TypeID` = ".intval($category['TypeID']);  $node->orderby = "GROUP BY `Title` ORDER BY `Title` DESC";  //////THIS IS WHERE I CAN ORDER THE WAY FIELDS ARE DISPLAYED IN ADD LISTING FORM////////////   if(($result = $mysql->select($node)) === false) {   die('Unable to display page fields');  }    $node = new sqlNode();  $node->table = "types";  $node->select = "*";  $node->where = "WHERE `ID` = ".intval($category['TypeID']);   if(($typeR = $mysql->select($node)) === false) {   die('Unable to Select Ad Type');  }   if(mysql_num_rows($typeR)<1) {   die("<script>window.location='/';</script>");  }   ///////////////Begin add form creation/////////////////////  echo "<h1>Create An Ad Within The<br>".$category['Title']."</h1>";  echo "<form id='generalform' class='container' action='add2.php' autocomplete='on' method='POST'>";  echo "<h3>Create Your Listing</h3>";  echo "<input type='hidden' name='PackageID' value='".$package['ID']."'>";   while($field = mysql_fetch_assoc($result)){   if($field['Type'] != "Mapping") {    echo "<div class='field'>    <label for".$field['Title'].">".$field['Title']."</div>";    switch($field['Type']) {	 case "text":	  if(($field['MaxChars']*1) > 0) {	   $maxlen = $field['MaxChars'];	  }	  echo "<input type='text' maxlength='$maxlen' name='".$field['Title']."' value='".$field['Default']."' validate='".$field['Validate']."' required='".$field['Required']."' message='".$field['Message']."'>";	  echo "<br><br>";	  break;	 	 case "textarea":	  if(($field['MaxChars']*1) > 0) {	   $max = $field['MaxChars'];	   $id = $field['Title'].'_message';	   $additional_attributes = " onkeyup='check_length($max,this,\"$id\");' onChange='check_length($max,this,\"$id\");' onmouseout='check_length($max,this,\"$id\");' ";	   echo "<div><span id='$id'>$max</span> Characters Left</div>";		  }   	  echo "<textarea placeholder='Provide a Detailed Description Here of Motorcycle or Parts Being Listed' $additional_attributes style='width:100%; height:150px;' name='".$field['Title']."'>".$field['Default']."</textarea>";	  break;	 	 case "select":	  echo "<select name='".$field['Title']."'>";	  $node = new sqlNode();	  $node->table = "customfields";	  $node->select = "*";	  $node->where = "WHERE `GroupID` = ".intval($field['GroupID']);	  $node->orderby = "ORDER BY `Default` DESC";		   if(($optionR = $mysql->select($node)) === false) {	    die();	   }	  	  while ($option = mysql_fetch_assoc($optionR)) {	   echo "<option value='".$option['Default']."' ";	    if($option['Selected'] == 'yes') {		 echo " Selected ";	    }	   	   echo " >";	 	   echo $option['OptionTitle'];	   echo "</option>";	  }	  echo "</select>";	  echo "<br><br>";	  break;	 	 case "checkbox":	  $node = new sqlNode();	  $node->table = "customfields";	  $node->select = "*";	  $node->where = "WHERE `GroupID` = ".intval($field['GroupID']);	  $node->orderby = "ORDER BY `Default` ASC";	 	 	  if(($optionR = $mysql->select($node)) === false) {	   die();	  }	 	  $num_checkboxes = mysql_num_rows($optionR);	  $count = 0;	  $break = ceil($num_checkboxes/2);	 	  while ($option = mysql_fetch_assoc($optionR)) {	   echo "<input type='checkbox' name='".$option['Title']."[]' value='".$option['Default']."' ";	   if($option['Selected'] == "selected") {	    echo "checked='checked'";	   }	   echo " >";	   echo $option['OptionTitle'];	   echo "<br>";	   if($count == $break) {	    echo "</td><td valign='top'>";	   }	   $count++;	  }		  break;  		 case "radio":	  $node = new sqlNode();	  $node->table = "customfields";	  $node->select = "*";	  $node->where = "WHERE `GroupID` = ".intval($field['GroupID']);	  $node->orderby = "ORDER BY `Default` ASC";	  	 	  if(($optionR = $mysql->select($node)) === false) {	   die($mysql->debugPrint());	  }	 	  $num_checkboxes = mysql_num_rows($optionR);	 	  $count = 0;	  $break = ceil($num_checkboxs/2);	 	  while ($option = mysql_fetch_assoc($optionR)) {	   echo "<input type='radio' name='".$option['Title']."' value='".$option['Default']."' ";	   if($option['Selected'] == "selected") {	    echo "checked='checked'";	   }	   echo " >";	   echo $option['OptionTitle'];	   echo "<br>";	   if($count == $break) {	    echo "</td><td valign='top'>";	   }	   $count++;	  } 	  break;	     }      }  }   echo "<input type='hidden' name='TypeID' value='".$category['TypeID']."'>";  echo "<input type='hidden' name='fatherID' value='$fatherID'>";  echo "<input type='submit' name ='submit' class='button' value='Create My Ad'>"; 	  echo "</td>";   echo "</tr>";   echo "</table>";     include('no-ad-footer.php'); }?>

Link to comment
Share on other sites

Well, it is really messy. And it's so long I really don't have time to inspect it. And I can't do a good job with that anyway, since I can't run it without your database. I do know that it parses without an error, but you already knew that. I don't see any obvious ways to clean it up. Part of me wants to combine a lot of those echo statements, but that's not a big deal. If it works, let it work?

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...