[RESOLVED] First PHP class for review/advice

jerry_louise

I have made a basic class that can take values from a form post, these can be in any combination and to any table and it will post the new record to the database.

Note: i have some validation functions i have not yet added.

the call on the php side

/************************* Post record *******************************/
	// Posts the record to the database
	$post = new newrecord;

	// Tbl Name
	$post->tbl_name = "forum-posts";

	// Send variables
	$post->postid = $_POST['postid']; // this is only used if it is something like a comment that relates to another object
	$post->subject = $_POST['subject'];
	$post->body = $_POST['body'];

	// Display output
	$post->PostToDatabase();

the class

<?php
/************************************************************************************
 								Jerry Roy
 								KittenbunnyCore
 								Database Class: Add new record 
************************************************************************************/

// Prevent direct access to file //
if(eregi(basename(__FILE__),$_SERVER['REQUEST_URI']))
die('<h1>Forbidden</h1><p>Direct access prohibited.</p>');

// Class deals with the following components

// --- Tbl --- //
// tbl_name

// --- DATA --- //
// PostID (relates for forum replys)
// Subject
// Body
// Date
// Poster

class newrecord 
{
	// Variables
	var $tbl_name;
	var $postid;
	var $subject;
	var $body;
	var $date;
	var $poster;

// Validate the Tbl_name
function tbl_name() 
{
	if (isset($this->tbl_name))
	{
 		return ' `'.$this->tbl_name.'`';
	}
  }

// Validate the PostID
function ValidatePostID() 
{
	if (isset($this->postid))
	{
			$valid_postid = $this->postid;
 		return $valid_postid;
	}
  }

// Validate the subject
function ValidateSubject() 
{
	if (isset($this->subject))
	{
 		return $this->subject;
	}
  }

// Validate the Body
function ValidateBody() 
{
	if (isset($this->body))
	{
 		return $this->body;
	}
  }

// Validate the Date
function ValidateDate() 
{
	if (isset($this->date))
	{
 		return $this->date;
	}
  }

// Validate the Poster
function ValidatePoster() 
{
	if (isset($this->poster))
	{
 		return $this->poster;
	}
}

// Make the sql string
function MakeSQL()
{

	// Varibles

	// Feilds
	$field['0'] = '`post_id`';
	$field['1'] = '`subject`';
	$field['2'] = '`body`';
	$field['3'] = '`date`';
	$field['4'] = '`poster`';


	// Values
	$value['0'] = $this->ValidatePostID();
	$value['1'] = $this->ValidateSubject();
	$value['2'] = $this->ValidateBody();
	$value['3'] = $this->ValidateDate();
	$value['4'] = $this->ValidatePoster();

	//******************** INSERT string *******************************//

	// Build the array
	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = $field[$x];
		}
		$x++;
	}

	// Build the array

	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));

	// Build the query

	// Start the query
	$start_return = '$sql = sprintf("INSERT INTO '.$this->tbl_name().'';

	// tables
	$field_return = ' ('.$queryStr.') ';
	//******************** INSERT string *******************************//

	//******************** Value string *******************************//

	// Values
	// Build the array
	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = '%s';
		}
		$x++;
	}
	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));
	$values_return = 'VALUES ('.$queryStr.')", ';
	//******************** Value string *******************************//

	//******************** Data String ********************************//

	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = mysql_real_escape_string($value[$x]);
		}
		$x++;
	}

	// Build the array

	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));	 
	$input_return = $queryStr;


	//******************** Data String ********************************//

	//Close
	$close_return = ');';


	// Build up the SQL String
	return $start_return . $field_return . $values_return . $input_return . $close_return;

}

// Return the results
  function PostToDatabase() 
	{
     	$query = mysql_query($this->MakeSQL());
			if(!$query)
			{
				echo "There was a error entering the data into the database, please report this issue to the site admin.";
			}
			else
			{
				echo "The post was successful";
			}
   	}

}

?>

dagon

are you really still using php 4 ?

jerry_louise

I have PHP 5 installed but am unaware of the differences between PHP 4 and 5 in coding i am just muddling along using books and tutorials for reference.

dagon

php 5 oo and php 4 oo have quite a few differences, you should only follow tutorials etc using 5

the manual has a section for each:

Classes and Objects (PHP 5)
Classes and Objects (PHP 4)

"Support for PHP 4 has been discontinued since 2007-12-31"

jerry_louise

Well i have only just started on classes and functions in the last month, by the looks of it it apears the only diffrence is public or private functions instead of just functions and public or private variables.

NogDog

As a quick comparison, just look at the contents for the classes/objects manual sections on PHP 4 versus PHP 5. PHP 5 provides many additions to objects/classes as well as a completely rewritten object engine underlying it all. You can't even use abstract classes or interfaces in PHP4, for a couple important examples besides the visibility feature you mentioned.

If you want to do any "serious" OOP in PHP, you definitely want to migrate to PHP5.

Now to take a look at your actual code....

jerry_louise

updated code

<?php
/************************************************************************************
 								Jerry Roy
 								KittenbunnyCore
 								Filename: newrecord.class.php
 								Database Class: Add new record 
************************************************************************************/

// Prevent direct access to file //
if(eregi(basename(__FILE__),$_SERVER['REQUEST_URI']))
die('<h1>Forbidden</h1><p>Direct access prohibited.</p>');

// Class deals with the following components

// --- Tbl --- //
// tbl_name

// --- DATA --- //
// PostID (relates for forum replys)
// Subject
// Body
// Date
// Poster

class newrecord 
{

// Variables
public $tbl_name;
public $postid;
public $subject;
public $body;
public $date;
public $poster;

// Validate the Tbl_name
private function tbl_name() 
{
	if (isset($this->tbl_name))
	{
 		return ' test.`'.$this->tbl_name.'`';
	}
  }

// Validate the PostID
private function ValidatePostID() 
{
	if (isset($this->postid))
	{
			$valid_postid = $this->postid;
 		return '$valid_postid';
	}
  }

// Validate the subject
private function ValidateSubject() 
{
	if (isset($this->subject))
	{
		$valid_subject = $this->subject;
 		return '$valid_subject';
	}
  }

// Validate the Body
private function ValidateBody() 
{
	if (isset($this->body))
	{
		$valid_body = $this->body;
 		return '$valid_body';		
	}
  }

// Validate the Date
private function ValidateDate() 
{
	if (isset($this->date))
	{
 		$valid_date = $this->date;
 		return '$valid_date';
	}
  }

// Validate the Poster
private function ValidatePoster() 
{
	if (isset($this->poster))
	{
 		$valid_poster = $this->poster;
 		return '$valid_poster';
	}
}

// Make the sql string
private function MakeSQL()
{

	// Varibles

	// Feilds
	$field['0'] = '`post_id`';
	$field['1'] = '`subject`';
	$field['2'] = '`body`';
	$field['3'] = '`date`';
	$field['4'] = '`poster`';


	// Values
	$value['0'] = $this->ValidatePostID();
	$value['1'] = $this->ValidateSubject();
	$value['2'] = $this->ValidateBody();
	$value['3'] = $this->ValidateDate();
	$value['4'] = $this->ValidatePoster();

	//******************** INSERT string *******************************//

	// Build the array
	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = $field[$x];
		}
		$x++;
	}

	// Build the array

	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));

	// Build the query

	// Start the query
	$start_return = '$sql = sprintf("INSERT INTO '.$this->tbl_name().'';

	// tables
	$field_return = ' ('.$queryStr.') ';
	//******************** INSERT string *******************************//

	//******************** Value string *******************************//

	// Values
	// Build the array
	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = '\'&#37;s\'';
		}
		$x++;
	}
	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));
	$values_return = 'VALUES ('.$queryStr.')", ';
	//******************** Value string *******************************//

	//******************** Data String ********************************//

	$x = 0;
	foreach ($field as $key)
	{
		if (isset($value[$x]))
		{
			$strArray[$x] = $value[$x];
		}
		$x++;
	}

	// Build the array

	$queryStr = ''; 
	$andStr = ', '; 

	// Build the string
	foreach($strArray as $str) 
	{ 
		if(!empty($str)) 
		{ 
			$queryStr = $queryStr . $str . $andStr; 
		} 
	}	 

	// Remove last $andStr added to str 
	$queryStr = substr($queryStr, 0, strlen($queryStr)-strlen($andStr));	 
	$input_return = $queryStr;


	//******************** Data String ********************************//

	//Close
	$close_return = ');';


	// Build up the SQL String
	return $start_return . $field_return . $values_return . $input_return . $close_return;

}

// Return the results

public function PostToDatabase() 
{
 	$query = mysql_query($this->MakeSQL());
		if(!$query)
		{
			echo "There was a error entering the data into the database, please report this issue to the site admin." . mysql_error();
		}
		else
		{
			echo "The post was successful";
		}
}

}

?>

Have not done much working on a issue that it is returning
There was a error entering the data into the database, please report this issue to the site admin.You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '$sql = sprintf("INSERT INTO test.forum-posts (post_id, subject) VALUES ('' at line 1

NogDog

This is by no means the best way or even the way I would choose to do it, but it might serve as an example of how to modularize things more, keeping each method narrowly focused on serving a single "mission" in the scheme of things. It also demonstrates how you could make use of the __set() "magic" method, just for the fun of it. Hopefully this will give you some ideas and lead you to some more things to think about.

<?php
class newrecord
{
   // Variables
   private $tbl_name;
   private $fields = array(
      'postid'    => null,
      'subject'   => null,
      'body'      => null,
      'date'      => null,
      'poster'    => null
   );

   /**
    * Set the table name
    * @param string $name
    * @return bool
    */
   public function set_tbl_name($name)
   {
      $this->tbl_name = trim($name);
      return (bool)$this->tbl_name; // false if set to empty string
   }

   /**
    * Set $fields values
    * Uses the __set() "magic" method
    * @param string $key
    * @param mixed $value
    * @return bool
    */
   public function __set($key, $value)
   {
      if(array_key_exists($key, $this->fields))
      {
         $this->fields[$key] = trim($value);
         return true;
      }
      else
      {
         user_error("Invalid field '$key'");
         return false;
      }
   }

   /**
    * Do the insert
    * @return bool
    */
   public function PostToDatabase()
   {
      $sql = $this->MakeSQL();
      $query = mysql_query($sql);
      if(!$query)
      {
         error_log(mysql_error()."\n$sql");
      }
      return (bool)$query;
   }

   /**
    * Create insert SQL string
    * @return string
    */
   private function MakeSQL()
   {
      $sql = sprintf(
         "INSERT INTO `&#37;s` (`%s`) VALUES(%s)",
         $this->tbl_name,
         $this->field_list(),
         $this->value_list()
      );
      return $sql;
   }

   /**
    * Create list of fields for use in SQL
    * @return string
    */
   private function field_list()
   {
      $fields = array();
      foreach($this->fields as $key => $val)
      {
         $fields[] = "`$key`";
      }
      return implode(',', $fields);
   }

   /**
    * Create list of values for use in insert SQL
    * @return string
    */
   private function value_list()
   {
      $values = array();
      foreach($this->fields as $value)
      {
         if(is_numeric($value))
         {
            $values[] = $value;
         }
         else
         {
            $values[] = "'".mysql_real_escape_string($value)."'";
         }
      }
      return implode(',', $values);
   }
}

Sample usage:

<?php
require_once 'newrecord.php';
mysql_connect('localhost', 'foo', 'bar');
$test = new newrecord();
$test->set_tbl_name('test');
$test->subject = 'test subject'; // uses "magic" __set() method -- cool, eh?
$test->body = 'test body';
// etc. for other fields
$test->PostToDatabase();

jerry_louise

Thanks Nogdog i am going to give it a good read though see how it works then write my new class.

Weedpacket

NogDog wrote:
As a quick comparison, just look at the contents for the classes/objects manual sections on PHP 4 versus PHP 5. PHP 5 provides many additions to objects/classes as well as a completely rewritten object engine underlying it all. You can't even use abstract classes or interfaces in PHP4, for a couple important examples besides the visibility feature you mentioned.

I think the most important difference is that objects are automatically passed by reference in PHP 5, instead of by value as in PHP 4. That brings them into line with how they're usually "expected" to behave - where if you change an object's property, that property actually changes.

class Foo
{
     var $bar = 42;
}

function pub_crawl($foo)
{
    $foo->bar = 17;
}

$foonly = new Foo;
pub_crawl($foonly);
echo $foonly->bar; // Different results in PHP 4 and PHP 5.

In PHP5, $foo and $foonly both refer to the same object (as you'd expect). In PHP 4, the object referred to by $foo is only a copy of the one referred to by $foonly.

laserlight

Weedpacket wrote:
I think the most important difference is that objects are automatically passed by reference in PHP 5, instead of by value as in PHP 4. That brings them into line with how they're usually "expected" to behave - where if you change an object's property, that property actually changes.

Again, I would like to stress that the use of the term "passed by reference" can be misleading because PHP's own reference mechanism performs aliasing that is different from what happens when an object is passed as an argument in PHP, which is more like passing a pointer. Consequently, it is only safe to change a reference parameter to some class type in PHP4 to a non-reference parameter in PHP5 if no assignment to the object itself is done within the function.

Weedpacket

Again, I was referring to changing the object's properties, not to replacing the object itself. I also wasn't referring to using aliases (which is what PHP's developers should have called what they chose to call "references"). They themselves admit that aliases are not like pointers - even though "pointer" and "reference" are effectively synonyms.

laserlight

Weedpacket wrote:
Again, I was referring to changing the object's properties, not to replacing the object itself. I also wasn't referring to using aliases (which is what PHP's developers should have called what they chose to call "references").

Precisely; it can be miscontrued by someone not familiar with the subtle difference. I suspect the term was selected by borrowing from C++, in which case it is probably originally Stroustrup's fault.

Weedpacket

C's usage is that "reference" refers to the concept while "pointer" is the implementation of that concept. PHP's concept is different but uses the same name.

The reference/pointer distinction is usually blurred in practice. Compare "Foo is a reference to bar" with "Foo is a pointer implementing a reference to bar". The former is technically the less accurate, but it's also more inuitive in the same way that "I hear a train" is less accurate but more intuitive than "I hear the sound of a train". In both cases, the extra accuracy introduces an extra level of indirection that doesn't provide any extra clarification (consider the number of levels of indirection that are hidden under "I hear the sound of a train").

Sorry, I've probably been reading a bit too much Bertrand Russell lately.

Nope, I blame PHP for the confusion. See? Now they've got two different kinds of reference - theirs, and everyone else's - and no adequate terminology to distinguish between them (unless they commit further abuse and describe the way objects are passed as "pass by pointer").

They should have used "alias" from the start, and even went to the trouble of explicitly stating that aliases were exactly what they were (to paraphrase: "PHP's references are not pointers, but symbol table aliases"). But they can't change now as that would break BC.

laserlight

Weedpacket wrote:
Nope, I blame PHP for the confusion. See? Now they've got two different kinds of reference - theirs, and everyone else's - and no adequate terminology to distinguish between them (unless they commit further abuse and describe the way objects are passed as "pass by pointer").

I will not fully absolve them of blame, but the reason why I say it is probably originally Stroustrup's fault is because the way that PHP references work in practice is identical to the way that C++ references work, with similiarities in the use of the ampersand (though assigning by reference follows the C and C++ address-of operator syntax without pointer notation rather than C++ reference syntax without the type name), even though the underlying implementation may be different.

Weedpacket wrote:
They should have used "alias" from the start, and even went to the trouble of explicitly stating that aliases were exactly what they were

Yes, and C++ programmers often say the same thing: a C++ reference is an alias.

Weedpacket wrote:
But they can't change now as that would break BC.

I do not think it will break backward compatibility since this is just terminology as the syntax does not use a "reference" keyword. Changing terminology now would be difficult because of the ingrained use of the term among PHP programmers to mean an alias.

Weedpacket

laserlight wrote:
Changing terminology now would be difficult because of the ingrained use of the term among PHP programmers to mean an alias.

Yup, that's what I meant.

The difference between PHP and C/C++ in all this of course is PHP's symbol table: a "reference" could refer to either a copy of a pointer into the symbol table (this being a normal PHP reference), or a copy of a pointer in the symbol table to the corresponding value (that being an "object reference" and more like C/C++'s concept). Without that extra layer there wouldn't be the potential for confusion (but then it wouldn't be PHP any more).