Accouting script

[move3rd]

I've written this script to alow artists selling at my site (electronic music store) to view sales stats.

Any advice on ways to improve it would be apreciated.

Is it safe for me to post this script? I suppose there is no connection to my site?

<?
 ////////// Start Account Summary
 if(!isset($_GET['accounts_a']) && !isset($_POST['accounts_a']))
 {
    $sql_inventory = "SELECT * ".
    "FROM `store_inventory` WHERE `cat_id`='".$_SESSION['artist_id']."'";
    $result_inventory = mysql_query($sql_inventory);

// Report Db errors
if (!$result_inventory) {
   $content = "Could not successfully run query ($sql) from DB: " . mysql_error();
}

while ($row_inventory = mysql_fetch_array($result_inventory))
{


   $title = $row_inventory['title'];
   $product = $row_inventory['product'];
   $promotion = $row_inventory['promotion'];
   $current_price = $row_inventory['price'];

   // Get Promotion

   $sql_promotions = "SELECT * ".
   "FROM `promotions` WHERE `name` = '$promotion'";
   $result_promotions = mysql_query($sql_promotions);

   // Report Db errors
   if (!$result_promotions){
      $content = "Could not successfully run query ($sql) from DB: " . mysql_error();
                           }

   while ($row_promotions = mysql_fetch_array($result_promotions))
            {
            $promo_description = $row_promotions['description'];
            }
   // End Get Promotion

   // Reset profit start value
   $release_profit = '0';
   //$balance = '0';


   // Calculate Balance
   $sql_balance ="SELECT SUM((price - promotion_fee) * quantity) AS balance".
   " FROM store_order_inv WHERE".
   " product='$product' AND `payroll_status` != 'Complete'".
   " AND status='Complete'";

   $result_balance = mysql_query($sql_balance);

   // Report Db errors
   if (!$result_balance) {
      $content = "Could not successfully run query ($sql) from DB: " . mysql_error();
                          }

   $row_balance = mysql_fetch_object($result_balance);
   $balance = number_format($balance + $row_balance->balance,2);


   // End Calculate Balance

   // Calculate Release Profit
   $sql_release_profit ="SELECT SUM(((price - promotion_fee) * quantity)) AS release_profit".
   " FROM store_order_inv WHERE".
   " product='$product' AND".
   " status='Complete'";

   $result_release_profit = mysql_query($sql_release_profit);

   // Report Db errors
   if (!$result_release_profit) {
      $content = "Could not successfully run query ($sql) from DB: " . mysql_error();
                                }

   $row_release_profit = mysql_fetch_object($result_release_profit);
   $release_profit = number_format($release_profit + $row_release_profit->release_profit,2);

   // End Calculate Release Profit

   // Calculate Release Sales
   $sql_release_sales ="SELECT SUM((quantity)) AS release_sales".
   " FROM store_order_inv WHERE".
   " product='$product' AND".
   " status='Complete'";

   $result_release_sales = mysql_query($sql_release_sales);

   // Report Db errors
   if (!$result_release_sales) {
      $content = "Could not successfully run query ($sql) from DB: " . mysql_error();
                               }

   $row_release_sales = mysql_fetch_object($result_release_sales);

   $release_sales = $row_release_sales->release_sales;

   // End Calculate Release Sales

   $total_sales = $total_sales + $release_sales; // Claculate total number of sales

   $total_profit = number_format($total_profit + $release_profit,2); // Claculate total release profit

   if(!$row_release_sales->release_sales){ $release_sales ="no sales yet";}

   $content .="<table width=\"100%\" cellspacing=\"0\" cellpadding=\"0\">".
   "<tr>".
         "<td >".
   "<img src=\"/images/gd_text/gd_title.php?text=$title Summary\" border=\"0\" alt=\"$title Summary\"/>".
         "</td>".
   "</tr>".
   "<tr>".
         "<td height=\"4\">".
         "</td>".
   "</tr>".
              "<table>".

              "<table width=\"100%\" cellspacing=\"0\" cellpadding=\"3\">".
   "<tr>".
          "<td class=\"title\" width=\"92\">".
   "<b>Product Code:</b>".
          "</td>".
          "<td>".
   "$product".
          "</td>".
   "</tr>".
   "<tr>".
          "<td class=\"title\">".
   "<b>Price:</b>".
          "</td>".
          "<td>".
   "$$current_price".
          "</td>".
   "</tr>".
   "<tr>".
          "<td class=\"title\">".
   "<b>Promotion:</b>".
          "</td>".
          "<td>".
   "$promotion ($$fee pcd)".
          "</td>".
   "</tr>".
   "<tr>".
          "<td>".
          "</td>".
          "<td>".
   "$promo_description".
          "</td>".
   "</tr>".
   "<tr>".
          "<td class=\"title\">".
   "<b>Sold:</b>".
          "</td>".
          "<td>".
   "$release_sales".
          "</td>".
   "</tr>".
   "<tr>".
          "<td class=\"title\">".
   "<b>Profit:</b>".
          "</td>".
          "<td>".
   "$$release_profit".
          "</td>".
   "</tr>".
   "<form action=\"index.php\" method=\"POST\">".
   "<tr>".
          "<td class=\"title\">".
          "</td>".
          "<td align=\"left\">".
   "<input name=\"a\" type=\"hidden\" value=\"accounts\">".
   "<input name=\"product\" type=\"hidden\" value=\"$product\">".
   "<input name=\"title\" type=\"hidden\" value=\"$title\">".
   "<input class=\"submit\" type=\"submit\" name=\"accounts_a\" value=\"Details\">".
          "</td>".
   "</tr>".
   "</form>".
   "<tr>".
          "<td height=\"4\">".
          "</td>".
   "</tr>".
   "</table>";

   }// End while - FROM `store_inv`

[Weedpacket]

Well, using SELECT * instead of SELECT [the fields you actually want] is ugly - it can hurt query optimisation techniques, and so cause it to run a bit slower than it otherwise might have. On top of that, it means you're transferring data from the database to the script that you don't need to. That takes time and memory as well.

echo <<<EOF;
For the big table at the end, the code would look a lot tidier using the Heredoc syntax (as illustrated by this paragraph). But note, too, that variable interpolation - whether using double-quoted strings or heredoc-quoted strings - is on the order of twice as slow as concatenating variables and strings together.
EOF;

Using something like "$promo_description" when $promo_description or '0' when 0 is intended leads to unnecessary typecasting, and is also a potential source of errors.

mysql_error() should not be used in production code; this is a security risk, as it effectively provides attackers with a debugger.

Why are you using mysql_fetch_array() sometimes and mysql_fetch_object() at others? (But then, I'm not too clear why the mysql_fetch_object() function was created in the first place.)

[move3rd]

Thanks weedpacket,

This code isnt in production yet. Since this is critical script I decided to post it hear before using it in production to make sure there where no serious problems with it.

mysql_error() should not be used in production code;

Seems really obovious now but I hadnt thought about this one... thanks

mysql_fetch_object() was sugested to me in thread hear for doing calculations within the sql query. Now I know that it's unesscery I will replace that with mysql_fetch_array().

echo <<<EOF;
For the big table at the end, the code would look a lot tidier using the Heredoc syntax (as illustrated by this paragraph). But note, too, that variable interpolation - whether using double-quoted strings or heredoc-quoted strings - is on the order of twice as slow as concatenating variables and strings together.
EOF;

So if I want the table in as the variable $content I could do this:

echo <<<content;
some test <table><tr><td></td></tr></table>
content;

yes?

[Weedpacket]

Originally posted by move3rd
So if I want the table in as the variable $content I could do this:

echo <<<content;
some test <table><tr><td></td></tr></table>
content;

yes? [/B]

Nyet;

$content = <<<EOF
some test <table><tr><td></td></tr></table>
EOF;

The EOF is basically a custom quote. EOF is traditional but you can use HOOTSMONWHERESMEGUILLEMET if you like. See here.

[move3rd]

I get no output when I do this:

       $content .= <<<EOF <table width="100%" cellspacing="0" cellpadding="0">
       <tr>
             <td >
       <img src="/images/gd_text/gd_title.php?text=$title Summary" border="0" alt="$title Summary"/>
             </td>
       </tr>
       <tr>
             <td height="4">
             </td>
       </tr>
                  <table>
EOF; 

The manual says I dont need to escape quotes... whats the problem?

[laserlight]

Keep the opening identifier separate from the string.

[move3rd]

Originally posted by laserlight
Keep the opening identifier separate from the string.

I also missed:

PHP Manual
The closing identifier must begin in the first column of the line.

I was expecting somone to comment on the fact that I do
4 database querys while looping through products in the
database, is this common practice? Would it not become a
strain on the server if there where 1000's of orders relating
to each product?