Jump to content
Larry Ullman's Book Forums

Recommended Posts

Hi All.

 

I am having a problem displaying results of my page, maybe I think because I've poorly written the code. I have these tables:

 

1. customers (cust_id, name, address, TIN, price, timestamp)

2. purchases(po_id, cust_id, delivery_address, delivered, receivedby, paid, encoderid, agentid, timestamp)

3. po_content(table_id, po_id, prod_id, ordered, delivered, srp, timestamp)

 

when generating a query, now that the database have a lot of content inside It's taking a while to display the result on the server and it shows "page cannot be displayed" while querying it from another computer via LAN. here's what I wrote:

 

$rownum = 1;

$a = "SELECT * FROM purchases ORDER BY timestamp ASC LIMIT $start $display";

$b = mysql_query($a);

while ($c = mysql_fetch_array($B)) {

       

        $d = "SELECT * FROM po_content WHERE po_id='{$c['po_id']}'";

        $e = mysql_query($d);

        while ($f = mysql_fetch_array($e)) {

                 $amount = $amount + ($f['srp'] * $f['delivered']); 

         }

         

        $total = $total + $amount;

         

        $g = "SELECT * FROM customers WHERE cust_id='{$c['cust_id']}'";

        $h = mysql_query($g);

        $i = mysql_fetch_array($h);

 

       

        //bg stuff so it would be seen nicely

        if ($rownum % 2 == 0 ) {

               $bgcolor = '"bgcolor=#f3f3f3"';

        } else {

               $bgcolor = '"bgcolor=#ffffff"';

        }

 

        print '

                <table width="100%" border="0" cellspacing="0" cellpadding="0" ' . $bgcolor . '>

                 <tr>
                     <td>' . $rownum . '. </td>
                      <td>' . $c['po_id'] . '</td>
                      <td>' . $i['name'] . '</td>
                      <td>' . date('m d, Y', $c['timestamp']) . '</td>
                      <td>$ ' . number_format($amount, 2) . '</td>
                      <td>' . $agent . '</td>
                      <td>' . $encoder . '</td>
                   </tr>
                  </table>

                ';

        $amount = 0; //reset the amount before looping again.

}

print '<td><div align="right">' . $number_format($total, 2) . '</div></td>';

 

There. $agent and $encoder has been called from an if(in_array) but still I think it adds up to the whole process. Now that the structure of the database has been like that. Would there be a damage control type thing that would optimize my queries? Many many thanks in advance!

Link to comment
Share on other sites

Hmmm... I'm not entirely sure what you're going for, but I'm definitely seeing some serious inefficiencies. While I don't know the sizes of your tables (i.e., how many records each one contains), you seem to be grabbing everything from the purchases table, and within that while loop, you are then grabbing everything from the other two tables where a certain value equals a certain value in the purchases table.

 

Also, a lot of the math you're doing (for example, adding up values for the amount of a given order) and formatting of the date can be done on the DB side, which can further speed things up.

In general, I think your goal should be to format all your data exactly the way you want to print it out to the screen by using one query. That may not be possible, and I'm not entirely sure what you want, but I'm thinking that we *may* be able to get everything you want in one query. I will attempt to do so below, but I can't guarantee that it'll work.

 

To start with, I'm trying really hard to figure out what exactly you're going for, but it's a bit abstract with the variables you're using. I'm not sure if you're using those variables on purpose to disguise your code for this thread, or if you're really using those variables in your code, but either way, I would definitely recommend using more logical variable names.

 

Anyway, here's my interpretation of your code:

You're printing out a table of purchase orders. The first column is the row number (which you do not seem to be properly incrementing within the outer while loop). The second column is the ID of a purchase order that a customer has made. The third column is the name of the customer (and it looks like you're storing their first and last name in one column in the customers table, which I would recommend against). The fourth column is the date and time of the purchase. The fifth column is the total amount of the purchase order formatted in dollars and cents. Lastly, I'm not sure what the last two columns are, but they aren't coming from DB data, so I'll ignore those for now.

 

Assuming my interpretation above is correct, I think you need to do an inner join across three tables with the purchases table being the main table. Also, I think you need to group your purchases together by the purchase ID, so that you can use an aggregate function to add up the price of the individual items within each specific order.

Does that make sense?

 

Anyway, here's the query that I'm *thinking* will work (but I can't guarantee that it does or that it's what you want):



SELECT c.cust_id, c.name AS cust_name, p.po_id AS order_num, p.cust_id, DATE_FORMAT(p.timestamp, '%m %d, %Y') AS date, o.po_id, SUM(o.delivered * o.srp) AS amount
FROM customers AS c, purchases AS p, po_content AS o
WHERE c.cust_id = p.cust_id AND p.po_id = o.po_id
GROUP BY o.po_id
ORDER BY p.timestamp ASC
LIMIT $start $display;


 

A few notes about the query:

1) For your query, a join is essential. Specifically, two inner joins on the purchases table is what you need. Joins are tricky at first, but they're essential for most DBs, so I'd recommend studying up on them.

 

2) Only select the columns you need. Using SELECT * for three separate queries is getting you a lot of data you don't need, and is very inefficient.

 

3) Use aliases (e.g., "AS c", "AS p", etc.) on the tables to make typing out the query shorter and easier. Also, aliases are essential for being able to easily reference the results of aggregate functions, formatted, dates, etc.

 

4) Format the timestamp on the SQL side using the DATE_FORMAT function. It's faster and easier. Also, give the formatted date an alias to make it easier to access on the PHP side. Here's more info on the DATE_FORMAT function:


 

5) I'd calculate the total amount of each order on the SQL side. To do so, you need to use the SUM aggregate function, and also use the GROUP BY clause to group your results together by order number so that you are adding up the correct grouping of items. Also, I'd assign an alias to the result of the SUM function.

 

6) The "ASC" part of the query is not necessary, since that's the default ordering. I left it anyway to avoid any further confusion.

 

7) I used "o" as the alias of the po_content table, as it seems like a table of orders to me.

 

That will hopefully handle the query side of things.

Unfortunately, I think there are some other issues with your code as well:

1) You're not incrementing $rownum in the while loop.

 

2) You're assigning your $bgcolor value to the entire table, not individual table rows. Also, I'm pretty sure the resulting HTML will be syntactically invalid and not work.

 

3) You're creating a new table each time through the while loop.

 

4) You're putting a div within a td for the total, which I wouldn't do.

 

Point being, without sounding too harsh, I think your code has some serious issues and needs some re-working. I get the feeling that you might be getting a bit too ambitious about your personal project without first understanding all the basics you need.

I don't mean to say that you shouldn't be ambitious, but I think you should probably go back to the book for a bit and bone up on queries with joins, HTML and PHP syntax, as well as think more about the logic of your while loop and the type of HTML that it is creating.

 

Anyway, below, I'm going to present the PHP I would use for your situation. Again, please keep in mind that I'm doing my best to piece together exactly what you want (and I'm not entirely sure), so I could be way off on this.



// I'm assuming that $agent and $encoder are already defined above.
 
$row_num = 1;
 
$total = 0;
 
$bg_color = '#FFF';
 
$q = "SELECT c.cust_id, c.name AS cust_name, p.po_id AS order_num, p.cust_id, DATE_FORMAT(p.timestamp, '%m %d, %Y') AS date, o.po_id, SUM(o.delivered * o.srp) AS amount FROM customers AS c, purchases AS p, po_content AS o WHERE c.cust_id = p.cust_id AND p.po_id = o.po_id GROUP BY o.po_id ORDER BY p.timestamp ASC LIMIT $start $display;";
// I'm assuming that $start and $display are already defined above.
 
$r = mysqli_query($dbc, $r);
// I am assuming that $dbc is already defined above. You also seem to have omitted this argument in your code.
 
echo '<table class="order_details">';
// I'd use CSS to properly format the table instead of the inline attributes you're using.
// As such, I have assigned a class to the table for that exact reason.
// Also, your table doesn't have any headers, but you may want to add them along with thead, th, tbody, and tfoot tags.
 
while ($row = mysqli_fetch_array($r, MYSQLI_ASSOC)) {
  
  echo '<tr style="background-color: ' . $bg_color . ';">
  
  <td>' . $row_num . '</td>
  
  <td>' . $row['order_num'] . '</td>
  
  <td>' . $row['cust_name'] . '</td>
  
  <td>' . $row['date'] . '</td>
  
  <td>' . number_format($row['amount'], 2) . '</td>
  
  <td>' . $agent . '</td>
  
  <td>' . $encoder . '</td>
  
  </tr>';
  
  $row_num++; // Don't forget to increment this.
  
  $total += $row['amount']; // This is the summation of the unformatted amounts, which could cause issues.
  
  $bg_color = ($row_num % 2 === 0) ? '#F3F3F3' : '#FFF'; // Ternary operation for brevity
  
}
 
echo '<tr style="background-color: ' . $bg_color . ';">
 
<td colspan="4" class="total_row">Total</td>
 
<td>' . number_format($total, 2) . '</td>
 
</tr>';
// Formatted to line up with the amount column.
// Also, note that I handled the total the same way you did, but if you calculate the total on the
// unformatted amount values, then you may get a discrepancy in which the amounts don't add up to the total.
// Also, again, I'd use CSS (not inline HTML) to align "Total" to the right.
// Lastly, you may want to put the total in a tfoot element.


 

Well, I think that's about it.

After all this writing, I really hope that I got close to what you wanted, and that this post is of some use.

Please let me know.

Thanks.

  • Upvote 3
Link to comment
Share on other sites

Hi Sir,

 

Thank you for your reply. I will study about JOINS and deeper MySQL functions that I always shy away from, I had an attitude before that I think I already learned the "only" things I need to know to make it work. Which results to inefficiencies. Working with JOINS and AS  will be very difficult for me but I sure need to give it a try.

 

Thank you sir.

Link to comment
Share on other sites

HartleySan's reply is really useful and you will find that if you take the time to learn joins and few other programming techniques that he suggests, your programming skills will benefit enormously.

 

I just want to add because HartleySan didn't include an explanation but he has used the new(ish) mysqli extension. The OP uses the old mysql version which is deprecated. There are minor differences so updating to the new extension should be painless but make sure to read up on them - mainly the differences are to do with the arguments that the function takes. Or you might want to consider updating to the more secure PHP Data Object.

 

 

  • Upvote 1
Link to comment
Share on other sites

 Share

×
×
  • Create New...