PHP Update (code broken) 1

[ljwilson]

I guess something must have changed in my servers PHP configuration. The webmaster indicated that they updated thier PHP install and now my program is not working. Here is my code:

#!/usr/bin/php
<?php

include("clsDatabase.php");
include("clsDebug.php");
$spaces = '...................................................';

function NewItem() {
  $db = new Database;
  $bug = new Debug;

  $itemdepartment = 'Rx ';
  if ( $w_OTC_Flag != '' ) { $itemdepartment = "HBA"; }
  if (( $w_OTC_Flag == '' ) and ( strlen($w_UPC) == 12 )) {
     $itemdepartment = "HBA";
  }
  echo "abc_update: Added $itemnumber\n";
  $sql = "INSERT INTO tblItems ("
       ."itemnumber, "
       ."itemdescription, "
       ."itemprice, "
       ."itemcost, "
       ."itemdepartment, "
       ."vendor, "
       ."qtyonhand, "
       ."value "
       .") VALUES ( "
       ."'".$itemnumber."', "
       ."'".$db->SQLFixup($itemescription)."', "
       ."'".$itemretail."', "
       ."'".$itemcost."', "
       ."'".$itemdepartment."', "
       ."'ABC',0,0.00 "
       .");";
  $rs = $db->query($sql);
} // function New_Item

function UpdateItem() {
  $db = new Database;
  $bug = new Debug;

          $CurrentItem = $db->fetch_array($rs);
          $UpdateItem  = $CurrentItem;
          if (( $CurrentItem['itemcost']  != $itemcost  ) or
            ( $CurrentItem['itemprice'] != $itemprice )) {
             echo "abc_update: Updated $itemnumber\n";
             $sql = "UPDATE tblItems set "
                  . "itemcost = $itemcost, "
                  . "itemprice = $itemprice "
                  . "WHERE itemNumber = '$itemnumber'";
             $db->query($sql);
          } // Update needed
} // function Update_Item

function ProcessUpdate( $filename ) {
  $db = new Database;
  $bug = new Debug;
  $spaces = '                                                ';

  if ( $filename == "" ) {
     $bug->Fatal("abc_update: File name not specified");
  } else {
     $fh = fopen($filename,'r') or
	$bug->Fatal("abc_update: Cannot open file: $filename");
  }

  $bug->Log("abc_update: Processing file: $filename");

  while(!feof($fh)) {

    $buf = fgetcsv($fh,0,"\t");

    // =========== Get the fields
    $vendoritemnumber = $buf[0]; 	// Vendor Item Number
    $itemdescription  = $buf[1]; 	// Description
    $itemcost         = $buf[2];	// Cost
    $itemAWP	      = $buf[3];	// AWP
    $vendorcode       = $buf[4]; 	// Code
    $itemNDC          = $buf[5]; 	// NDC
    $itemUPC          = $buf[6]; 	// UPC
    $item_GCN_SEQNO   = $buf[7]; 	// GCN_SEQNO
    $itemMAC          = $buf[8]; 	// MAC
    $itemOTC_Flag     = $buf[9]; 	// OTC Flag
    $itemprice        = $buf[10];	// Retail Price

    // Set the Item Number
    if ($itemUPC != "" ) {
       $itemnumber = $itemUPC;	// Prefer UPC Code
    } else {
       $itemnumber = $itemNDC;	
    }

    if ( $itemnumber == "" ) {
       $msg = "abc_update: No Item Number for $itemdescription";
       $bug->Log($msg);
    } else {
      // Check to see if the item is already in the database
      $sql = 'SELECT * FROM tblItems WHERE itemNumber = "'.$itemnumber.'"';
      $rs = $db->Query($sql);
      $item = $db->fetch_array($rs);

      if ( $item['itemprice'] != $itemprice ) {
         $fill = 40 - strlen($item['itemdescription']);
         echo $item['itemdescription']
	    . substr($spaces,1,$fill)
            . '  Old Retail: '.number_format($item['itemprice'],2,'.',',')
            . '  New Retail: '.number_format($itemprice,2,'.',',')
            . "\n";
      }
      if ( $rs ) { // Update
	UpdateItem();
      } else { // New item
 	NewItem();
     }
    }
  } // while not eof
  fclose($fh);
} // function ProcessUpdate file.

// Main 
ProcessUpdate("/var/spool/uucppublic/OTC.txt");

?>

Specifically, the $itemprice, $itemcost, $itemnumber and the $rs variables aren't showing a value when I log the value in the UpdateItem function. It seems like a problem with variable scope, but this was working before. Any ideas?

LJ

 

[Stretchwickster]

It's very likely that your code worked because the register_globals directive was set to on in your previous version of PHP. This feature allowed PHP to create a global variable for every form field and cookie. As of PHP 4.2.0, due to the associated security risks this presented, this directive defaulted to off. So, I'm guessing that $itemprice, $itemcost and $itemnumber correspond to fields in a form - am I correct?

The correct approach is to use one of the following superglobal arrays: $_REQUEST, $_GET or $_POST.
For example, $_REQUEST['itemcost'] would correctly reference a form field called 'itemcost'.

Clive

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"To err is human, but to really foul things up you need a computer." (Paul Ehrlich)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
To get the best answers from this forum see: faq102-5096

 

[ljwilson]

No, they were not fields in a form, they were values read in from a CSV file. I am pretty sure that REGISTER_GLOBALS was off though.

Any other thoughts?

Thanks,

LJ

 

[Stretchwickster]

Apologies, I misread your code. It seems to me that it's a problem with variable scoping. As far as I can see, $itemprice, $itemcost, $itemnumber and $rs are only locally defined within ProcessUpdate. Therefore, you need to define any variables to be used globally outside of any functions, then within each function you can import the global variable into the function's scope as follows:

...
[b]// assuming the variables you wish to use globally have been declared/populated previously...[/b]

function UpdateItem() {
  [b]global $itemcost, $itemprice, $itemnumber, $rs;[/b]
  $db = new Database;
  $bug = new Debug;

          $CurrentItem = $db->fetch_array($rs);
          $UpdateItem  = $CurrentItem;
          if (( $CurrentItem['itemcost']  != $itemcost  ) or
            ( $CurrentItem['itemprice'] != $itemprice )) {
             echo "abc_update: Updated $itemnumber\n";
             $sql = "UPDATE tblItems set "
                  . "itemcost = $itemcost, "
                  . "itemprice = $itemprice "
                  . "WHERE itemNumber = '$itemnumber'";
             $db->query($sql);
          } // Update needed
} // function Update_Item

function ProcessUpdate( $filename ) {
  [b]global $itemcost, $itemprice, $itemnumber, $rs;[/b]
  $db = new Database;
  $bug = new Debug;
  $spaces = '                                                ';

  if ( $filename == "" ) {
     $bug->Fatal("abc_update: File name not specified");
  } else {
     $fh = fopen($filename,'r') or
    $bug->Fatal("abc_update: Cannot open file: $filename");
  }

  $bug->Log("abc_update: Processing file: $filename");

  while(!feof($fh)) {

    $buf = fgetcsv($fh,0,"\t");

    // =========== Get the fields
    $vendoritemnumber = $buf[0];     // Vendor Item Number
    $itemdescription  = $buf[1];     // Description
    $itemcost         = $buf[2];    // Cost
    $itemAWP          = $buf[3];    // AWP
    $vendorcode       = $buf[4];     // Code
    $itemNDC          = $buf[5];     // NDC
    $itemUPC          = $buf[6];     // UPC
    $item_GCN_SEQNO   = $buf[7];     // GCN_SEQNO
    $itemMAC          = $buf[8];     // MAC
    $itemOTC_Flag     = $buf[9];     // OTC Flag
    $itemprice        = $buf[10];    // Retail Price

    // Set the Item Number
    if ($itemUPC != "" ) {
       $itemnumber = $itemUPC;    // Prefer UPC Code
    } else {
       $itemnumber = $itemNDC;    
    }

    if ( $itemnumber == "" ) {
       $msg = "abc_update: No Item Number for $itemdescription";
       $bug->Log($msg);
    } else {
      // Check to see if the item is already in the database
      $sql = 'SELECT * FROM tblItems WHERE itemNumber = "'.$itemnumber.'"';
      $rs = $db->Query($sql);
      $item = $db->fetch_array($rs);

      if ( $item['itemprice'] != $itemprice ) {
         $fill = 40 - strlen($item['itemdescription']);
         echo $item['itemdescription']
        . substr($spaces,1,$fill)
            . '  Old Retail: '.number_format($item['itemprice'],2,'.',',')
            . '  New Retail: '.number_format($itemprice,2,'.',',')
            . "\n";
      }
      if ( $rs ) { // Update
    UpdateItem();
      } else { // New item
     NewItem();
     }
    }
  } // while not eof
  fclose($fh);
} // function ProcessUpdate file.

// Main
ProcessUpdate("/var/spool/uucppublic/OTC.txt");

?>

Alternatively, you can use the $GLOBALS superglobal array. See

http://uk3.php.net/global

for more info.

Clive

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"To err is human, but to really foul things up you need a computer." (Paul Ehrlich)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
To get the best answers from this forum see: faq102-5096

 

[ljwilson]

That is the answer I was expecting, I actually changed the code to use the if/else structure in place of the functions being called and just moved the code in the functions to the if/else structure.

I thought it would be a little cleaner that way and would keep me from having to use globals (I was using the $GLOBALS array for my values before I just moved the function code).

I guess my real question was: When did this stop working in PHP? I didn't write the code initially, but inherited it and started getting notifications last week about the prices not getting updated properly. I then got a call yesterday from someone with a specific example and sure enough, the price in the CSV didn't match my database for the item.

I then started looking for issues in the script and started logging values at different points and noticed that (just like what you mentioned) the values for $itemcost, etc weren't being passed from function to function. It was then that I just restructured it (after considering using the $GLOBALS array) and it works now.

Just wondering what/when something changed in PHP to cause it to break, so that I can tell me boss exactly why it stopped working.

Thanks so much,

LJ

 

[Stretchwickster]

I'm relatively new to PHP, so don't have much experience of how things used to be, and therefore cannot tell you what changed and when. Maybe someone else can offer an explanation? Just out of interest, what are the version numbers of the PHP installations you moved from and to?

You might find this article interesting reading:

http://www.hardened-php.net/globals-problem

My personal preference would be to pass the above variables as function arguments rather than using global variables, as you can always be sure which variable you are referring to!

Sorry I can't be of more assistance.

Clive

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"To err is human, but to really foul things up you need a computer." (Paul Ehrlich)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
To get the best answers from this forum see: faq102-5096

 

[DonQuichote]

function UpdateItem() {
  global $itemcost, $itemprice, $itemnumber, $rs;

Why don't you make the variables parameters to the function? Like:

function UpdateItem($itemcost, $itemprice, $itemnumber, $rs) {

You would then have to call the function with the parameter values also. This approach has a few advantages: you pass the values directly to the function instead of hoping nothing else has changed the global ones in the meantime. Depending on the editor you use, you may also see the parameters in autocomplete or in a function tree, so you know what the function expects. Your ProcessUpdate does this with $filename, but not with the variables you added.

+++ Despite being wrong in every important aspect, that is a very good analogy +++
Hex (in Darwin's Watch)

 

[ljwilson]

Actually, I thought this was cleaner:

function ProcessUpdate( $filename ) {
  $db = new Database;
  $bug = new Debug;
  $spaces = '                                                ';

  if ( $filename == "" ) {
     $bug->Fatal("abc_update: File name not specified");
  } else {
     $fh = fopen($filename,'r') or
	$bug->Fatal("abc_update: Cannot open file: $filename");
  }

  $bug->Log("abc_update Processing file: $filename");

  while(!feof($fh)) {

    $buf = fgetcsv($fh,0,"\t");

    // =========== Get the fields
    $vendoritemnumber = $buf[0]; 	// Vendor Item Number
    $itemdescription  = $buf[1]; 	// Description
    $itemcost         = $buf[2];	// Cost
    $itemAWP	      = $buf[3];	// AWP
    $vendorcode       = $buf[4]; 	// Code
    $itemNDC          = $buf[5]; 	// NDC
    $itemUPC          = $buf[6]; 	// UPC
    $item_GCN_SEQNO   = $buf[7]; 	// GCN_SEQNO
    $itemMAC          = $buf[8]; 	// MAC
    $itemOTC_Flag     = $buf[9]; 	// OTC Flag
    $itemprice        = $buf[10];	// Retail Price

    // Set the Item Number
    if ($itemUPC != "" ) {
       $itemnumber = $itemUPC;	// Prefer UPC Code
    } else {
       $itemnumber = $itemNDC;	
    }

    if ( $itemnumber == "" ) 
	{
       $msg = "abc_update: No Item Number for $itemdescription";
       $bug->Log($msg);
    } 
	else 
	{
    	// Check to see if the item is already in the database
        $sql = 'SELECT * FROM tblItems WHERE itemNumber = "'.$itemnumber.'"';
        $rs = $db->Query($sql);
        $item = $db->fetch_array($rs);

        if ( $item['itemprice'] != $itemprice ) 
		{
        	$fill = 40 - strlen($item['itemdescription']);
         	echo $item['itemdescription']
	    	. substr($spaces,1,$fill)
            . '  Old Retail: '.number_format($item['itemprice'],2,'.',',')
            . '  New Retail: '.number_format($itemprice,2,'.',',')
            . "\n";
    	}
	if ( $rs ) 
	{ // UpdateItem
	
	     $CurrentItem = $db->fetch_array($rs);
             $UpdateItem  = $CurrentItem;
             if (( $CurrentItem['itemcost']  != $itemcost  ) or ( $CurrentItem['itemprice'] != $itemprice )) 
	     {
        	echo "abc_update: Updated $itemnumber\n";
                $sql = "UPDATE tblItems set "
                  . "itemcost = $itemcost, "
                  . "itemprice = $itemprice "
                  . "WHERE itemNumber = '$itemnumber'";
                $db->query($sql);
             } // Update needed	
        }  // UpdateItem
	else  // AddItem
	{ 
             $itemdepartment = 'Rx ';
  	     if ( $w_OTC_Flag != '' ) 
	     { 
		$itemdepartment = "HBA"; 
	     }
		
  	     if (( $w_OTC_Flag == '' ) and ( strlen($w_UPC) == 12 )) 
	     {
     		$itemdepartment = "HBA";
  	     }
		
  	     echo "abc_update: Added $itemnumber\n";
  	     $sql = "INSERT INTO tblItems ("
		   ."itemnumber, "
		   ."itemdescription, "
		   ."itemprice, "
		   ."itemcost, "
		   ."itemdepartment, "
		   ."vendor, "
		   ."qtyonhand, "
		   ."value "
		   .") VALUES ( "
		   ."'".$itemnumber."', "
		   ."'".$db->SQLFixup($itemescription)."', "
		   ."'".$itemretail."', "
		   ."'".$itemcost."', "
		   ."'".$itemdepartment."', "
		   ."'ABC',0,0.00 "
		   .");";
	     $rs = $db->query($sql);
	
	  }  // else - AddItem
    }
  } // while not eof
  fclose($fh);
} // function ProcessUpdate file.

This way, I don't have to pass off control to another function and don't have to worry about globals either.

See any problems with this method?

LJ