Jump to content

Advise please on the best way to sort this:


roldahayes

Recommended Posts

Hi,

 

Please have a look at this code that I have - it is the results page of a "make" & "model" vehicle search.  It displays the product results in a table in sections depending on the case that has been entered to populate the database.

 

I am trying to clean up all of this code and incorporate it into Divs instead of tables.

 

My question is how would I go about structuring this to display the results in different divs rather than the coloured tables that it is now doing.

 

Any help would be great.

 

<?php //info bars colour$color_info="#D6D685"; // Almost Silver //FFFFCC$color1="#E9E9E9";// Light Blue$color2="#EAD5FF";// Light Blue  include_once("func_lib.php");include_once("usr_conn.php");if (!connectDB()){echo "<p>Unable To Connect To Database</p>";return;}if (($_GET ['make'] == "MAKE") or ($_GET ['model'] == "MODEL")){$errMessage =  "<br><br><b><font class=small>Your search hasn't returned any results, please re-enter your search again.</font></b><br><br><a href=van_accessories.php>Search again</a><br><br>"; }else{$strMake = mysql_escape_string($_GET ['make']);$strModel = mysql_escape_string($_GET ['model']);}$strDefaultProd = "XD";//write query string that will pased in paging links$strQuery = "make=".$strMake."&model=".$strModel."&";//echo $strQuery;$page = mysql_escape_string($_GET ['page']);$sqlSelect = "SELECT Prod_ID, Prod_Type, Prod_Model, Prod_Make, Prod_REF, Product_Desc, Price_ExVat, image_name FROM products";$criteria = " WHERE (Car_Make = '".$strMake."' AND Car_Model = '".$strModel."' AND Default_Code = 'CV') OR (Car_Make = '".$strMake."' AND Car_Model = '".$strModel."' AND Default_Code = 'LE') OR (Car_Make = 'all' AND Default_Code = 'RA')";$sqlquery = $sqlSelect . $criteria; $result = mysql_query($sqlquery);$count = mysql_num_rows($result);$full_count = $count; $records_per_page = 999;$total_pages = ceil( $count / $records_per_page );if ( $page == "" ) { $page = 1; }if ( $page == 1 ) {    $naviagation = "<font color=\"#666666\">First</font> | <font color=\"#666666\">Prev</font> | "; } else {     $prev_page = $page - 1;    $navigation = "<a href=\"van_roof_rack_results.php?".$strQuery."page=1\">First</a> | <a href=\"van_roof_Rack_results.php?".$strQuery."page=".$prev_page."\">Prev</a> | ";}if ( $page == $total_pages ) {} else {     $next_page = $page + 1;    $navigation .= "<a href=\"van_roof_rack_results.php?".$strQuery."page=".$next_page."\">Next</a> | <a href=\"van_roof_rack_results.php?".$strQuery."page=".$total_pages."\">Last</a>";}$offset = ( $page - 1 ) * $records_per_page;$sqlSelect = "SELECT Car_Make, Car_Model, Prod_ID, Prod_Type, Prod_Model, Prod_Make, Priority, Prod_Code, Prod_REF, Product_Desc, Price_ExVat, Link, image_name FROM products";$criteria = " WHERE (Car_Make = '".$strMake."' AND Car_Model = '".$strModel."' AND Default_Code = 'CV') OR (Car_Make = '".$strMake."' AND Car_Model = '".$strModel."' AND Default_Code = 'LE') OR (Car_Make = 'all' AND Default_Code = 'RA')"; $limit = " ORDER BY Priority ASC, Prod_Code ASC, Prod_ID ASC LIMIT $offset, $records_per_page";$sqlquery = $sqlSelect . $criteria . $limit ;$result = mysql_query($sqlquery);if (!$result || (mysql_num_rows($result) == 0)){}else{$count = mysql_num_rows($result); }?> <h1><?php echo("<font class=vehicle> " . $strMake . " " . $strModel ."</p>"); ?></h1>  <!--Start of results code -->                                               <?php     $title .= "</tr>"; //set the counters for the records results display screen$cnt = 0;$headercount1 = 0;$headercount2 = 0;$headercount3 = 0;$headercount4 = 0;$headercount5 = 0;$headercount6 = 0;$headercount7 = 0;while ($row = mysql_fetch_assoc($result)){$strLength = strlen ($row["Prod_Type"]);$strPrefix = substr($row["Prod_Type"], 0, 2);$strSuffix = substr($row["Prod_Type"], 2, $strLength);echo "<tr align=center><td colspan=7 class=small>";  //set bg cell color diff for BH makeswitch ($strPrefix){case "BH":$color=$color2;break; default:$color=$color1;break;}//end switch  switch ($strPrefix){   case "BH":        if ($headercount1 == 0) { echo "<img border=0 src=bulkheads.jpg alt=Bulkheads></td></tr>";echo $title;}$headercount1 ++; //$headercount1        break;   case "RB":       if ($headercount2 == 0){echo "<img border=0 src=roofracks.jpg alt=Roof-Bars></td></tr>";echo $title;} $headercount2 ++; //$headercount1       break; case "BX":       if ($headercount3 == 0){ echo "<img border=0 src=bulkheads.jpg alt=Roof-Boxes></td></tr>";echo $title;}$headercount3 ++; //$headercount1       break;case "BR":         if ($headercount4 == 0) { echo "<img border=0 src=bulkheads.jpg alt=Bike-Racks></td></tr>"; echo $title; }  $headercount4 ++; //$headercount1       break;case "SR":       if ($headercount5 == 0) {echo "<img border=0 src=bulkheads.jpg alt=Ski-Rack></td></tr>";echo $title;} $headercount5 ++;  //$headercount1        break;case "CR":        if ($headercount6 == 0) {echo "<img border=0 src=bulkheads.jpg alt=Canoe-Rack></td></tr>";   echo $title;}  $headercount6 ++; //$headercount1       break; case "XD":  if ($headercount7 == 0) {  echo "<img border=0 src=general.jpg alt=General-Accessories></td></tr>";echo $title; }   $headercount7 ++;  }//print a product row if the prodtype isn't set to INFO$pop_link = "";if ($strSuffix != "INFO"){//test to see if an image exists for the productif ($row["Link"] <> NULL){$pop_link = "   <a href=\"products/".$row["Link"]."?iframe=true&width=650&height=500\" rel=\"prettyPhoto\" >More information >>>></a>";}?>                                                                                                                                                                                      <?PHP if (isset($errMessage)){echo $errMessage;}if (!$result || (mysql_num_rows($result) == 0))echo ("<b><font class=small>Product Tyoe Not Found For Your Vehicle.</font></b><br><br>");// display the resultselse{// use rowcount to display total results found//*debugecho $navigation;}?>                                               <?php   $title =  "<tr align=\"center\" class=\"headertable\" height=\"32\">";  $title .= "<td width=\"2%\"><img src=\"images/transparent.gif\" width=\"95\" height=\"1\" /></td>";     $title .= "<td width=\"2%\">Make</td>";       $title .= "<td width=\"1%%\">Ref No.</td>";       $title .= "<td width=\"1%\">Title/Description</td>";       $title .= "<td width=\"1%\">Price Ex VAT</td>";       $title .= "<td width=\"73\">Price Inc VAT</td>";       $title .= "<td width=\"60\"><img src=\"2003/basket.gif\" width=\"21\" height=\"14\"></td>";       $title .= "</tr>"; $cnt = 0;$headercount1 = 0;$headercount2 = 0;$headercount3 = 0;$headercount4 = 0;$headercount5 = 0;$headercount6 = 0;$headercount7 = 0;while ($row = mysql_fetch_assoc($result)){$strLength = strlen ($row["Prod_Type"]);$strPrefix = substr($row["Prod_Type"], 0, 2);$strSuffix = substr($row["Prod_Type"], 2, $strLength);echo "<tr align=center><td colspan=7 class=small>"; //set bg cell color diff for BH makeswitch ($strPrefix){case "BH":$color=$color2;break; default:$color=$color1;break;}//end switch  switch ($strPrefix){   case "BH":        if ($headercount1 == 0) { echo "<img border=0 src=bulkheads.jpg alt=Bulkheads></td></tr>";echo $title;}$headercount1 ++; //$headercount1        break;   case "RB":       if ($headercount2 == 0){echo "<img border=0 src=roofracks.jpg alt=Roof-Bars></td></tr>";echo $title;} $headercount2 ++; //$headercount1       break; case "BX":       if ($headercount3 == 0){ echo "<img border=0 src=bulkheads.jpg alt=Roof-Boxes></td></tr>";echo $title;}$headercount3 ++; //$headercount1       break;case "BR":         if ($headercount4 == 0) { echo "<img border=0 src=bulkheads.jpg alt=Bike-Racks></td></tr>"; echo $title; }  $headercount4 ++; //$headercount1       break;case "SR":       if ($headercount5 == 0) {echo "<img border=0 src=bulkheads.jpg alt=Ski-Rack></td></tr>";echo $title;} $headercount5 ++;  //$headercount1        break;case "CR":        if ($headercount6 == 0) {echo "<img border=0 src=bulkheads.jpg alt=Canoe-Rack></td></tr>";   echo $title;}  $headercount6 ++; //$headercount1       break; case "XD": if ($headercount7 == 0) {  echo "<img border=0 src=general.jpg alt=General-Accessories></td></tr>";echo $title; }   $headercount7 ++;  }  //print a product row if the prodtype isn't set to INFO   $pop_link = "";if ($strSuffix != "INFO"){//test to see if an image exists for the productif ($row["Link"] <> NULL){$pop_link = "   <a href=\"products/".$row["Link"]."?iframe=true&width=650&height=500\" rel=\"prettyPhoto\" >More information >>>></a>";}echo "<tr class=stdtable align=center height=25>";  //DISPLAY THUMB IMAGE IF ONE WAS LISTED IN THE image_name FIELD$p_image_name = htmlspecialchars($row["image_name"]);//echo "p_image_name:$p_image_name";if ($p_image_name != "")echo "<td bgcolor=$color width=\"95\"><img src=\"images/product_images/$p_image_name\" border=\"0\" /></td>";elseecho "<td bgcolor=$color width=\"95\"><img src=\"images/transparent.gif\" width=\"95\" height=\"50\" border=\"0\" /></td>";echo "<td bgcolor=$color width=\"60\">". htmlspecialchars($row["Prod_Make"]) ."</td><td bgcolor=$color width=\"60\">" . htmlspecialchars($row["Prod_REF"]) . "</td><td bgcolor=$color align=left class=\"infolink\" width=\"550\">" . htmlspecialchars($row["Product_Desc"]) .$pop_link . "</td><td bgcolor=$color width=\"73\">£" . number_format(htmlspecialchars($row['Price_ExVat']), 2) ."</td><td bgcolor=$color width=\"73\">£" . number_format((calcVAT (htmlspecialchars($row['Price_ExVat']))), 2) ."</td><td class=\"btn btn-default btn-sm\"><a href=basket.php?src=".urlencode($_SERVER['REQUEST_URI'])."&productID=" . $row["Prod_ID"] . ">BUY</a></td></tr>";}// if the prod type is set to INFO then print out a new row for that informationelse { echo "<tr class=stdtable align=center height=25 ><td colspan=7 bgcolor=$color_info><em><strong>" . htmlspecialchars($row["Product_Desc"]) . "</strong></em></td></tr>";  } } ?> <?phpmysql_free_result($result);mysql_close();}}?>

 

 

Link to comment
Share on other sites

we don't really know what you want to do different. can you specifically show an example of what part of the data and exactly how you want it to be done differently?
 
also, this code has several problems that you should fix or change first. the two biggest ones are you should be using css to style everything, not inline styling, and your two nested while(){} loops, that are looping over the same data. by having these two nested loops, the first one is really only acting like an if() conditional statement to run the rest of the code. however, by having these nested loops, the first row of your result set is not being displayed, because the nested loop that's actually displaying the data, is fetching and starting at the second row in the result set.

 


 

here's a general suggestion that will help reduce that huge switch/case statement, by eliminating the repeated logic and converting the design into a data driven one, rather than a hard-code logic one, and make it easier to change or fix the html markup, because any particular piece of markup will only exist once in the code.
 

define the type information -

// define the type information (typically this would be stored in a 'config.php' include file or retrieved from a database table)
$types['BH'] = array('img'=>'bulkheads.jpg','alt'=>'Bulkheads');
$types['RB'] = array('img'=>'roofracks.jpg','alt'=>'Roof-Bars');
// ... repeat for all product types

initialization, before the start of the while(){} loop -

$last_type = ''; // initialize this before the start of your ONE while(){} loop that's display the data

at the point where you want to output the product type image inside the loop -

// output the product type image each time it changes while displaying the products
if($last_type != $strPrefix){
    // the type changed, output the image
    $img = $types[$strPrefix]['img'];
    $alt = $types[$strPrefix]['alt'];
    echo "<img border='0' src='$img' alt='$alt'></td></tr>"; // since this markup and code only exists once, if you are changing something about this section of code, you only have to make the change in one place
    echo $title;
    $last_type = $strPrefix; // remember the new value
}
Link to comment
Share on other sites

This code was written years ago so I am now trying to sort the styling out with CSS and removing all the inline stuff.  Also, there is so much code that as you've advised, needs streamlining......

 

 

 

Ok, I have made a quick example image of what I am trying to achieve,

 

The database is populated by uploading a CSV file.

 

Each section of the results has a unique code (RB, BH, RA etc)

 

I want each section to appear in the divs as in the picture (div-RB, div-BH, div-RA)

 

 

 

divs.jpg

Link to comment
Share on other sites

i recommend to just rewrite the presentation portion of the code to produce the output you want. the example code i posted above is actually more relevant to this than just the switch/case statement. by detecting the change in the RB, BH, RA type, the code can finish off any html for the previous section and start a new section. the code that displays the data just goes right after the if(){} logic i posted.

 

i do have another general suggestion. separate the business logic from the presentation logic. the business logic contains the php code that figures out what to do on the page and retrieves any data that's need to display the page. the result of the business logic should just be php variables. all the database specific statements should be in the business logic (you would pre-retrieve database data and store it in an array) and there should be no html markup in the business logic. the presentation logic should contain no database specific code, since it will receive any data it needs in php variables. the presentation logic should contain all the html/css/javascript.

Link to comment
Share on other sites

here is the sample code i posted above, with more details showing how it would control the building of the sections - 

// the business logic retrieves and stores the query results in an array $data. this serves as the input to this section of ocde
 if(empty($data)){
    // there's no matching data, output an appropriate message here...


} else {
    // there is data, start producing the output...


    $last_type = ''; // initialize this before the start of your loop that's display the data
    foreach($data as $row){
        $strPrefix = substr($row["Prod_Type"], 0, 2);
        // output the product type heading each time it changes
        if($last_type != $strPrefix){
            // the type changed
            if($last_type !=''){
                // not the first section, close out any previous section here...
                // close the </div> for example
                
            }
            // start a new section here.... start the <div>, output any type image, ....
        
            $img = $types[$strPrefix]['img'];
            $alt = $types[$strPrefix]['alt'];
            echo "<img border='0' src='$img' alt='$alt'>";
            $last_type = $strPrefix; // remember the new value
        }
        // output the data in each recored here....
        
    }
    // you would close the last section here. whatever logic you have in the if($last_type !=''){ ... } above would be repeated or in a user written function that you call


}
Edited by mac_gyver
Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.