Php Error Messaging

[designanddev]

I have this code here below but i would like to display errors so if the user doesn't fill out the specific fields they will get a message to fill that in, i can only figure out how to make it display one error message. heres the code, you guys have been great so far, sorry for duplicating my messages.

 

Kind Regards

 

Submit form.....

  if(isset($_POST['submit'])) {
  $first_name = trim($_POST['first_name']);
  $last_name = trim($_POST['last_name']);

  $new_user = User::Reg($first_name, $last_name);
  if($new_user && $new_user->save()) {
 redirect_to("photo.php?id={$photo->id}");

 } else {
  // Failed
 $message = "There was an error that prevented the comment from being saved.";
 }
} else {
 $first_name = "";
 $last_name = "";
}

 

This Reg function.....

 

public static function Reg($first_name, $last_name) {
   if(!empty($first_name) && !empty($last_name)) {
  $register = new User();
 $register->first_name = $first_name;
 $register->last_name = $last_name;
 return $register;
 } else {
  return false;
 }
}

[Muddy_Funster]

Within the Reg method you would need to either change your else to a couple of elseif 's or add another if inside the the else to check whcih of the fields was ommited and return an appropriate response deppending on which field(s) are left out.

[designanddev]

something like this... But its not working

 

public static function Reg($first_name, $last_name) {
   if(!empty($first_name) && !empty($last_name)) {
  $register = new User();
 $register->first_name = $first_name;
 $register->last_name = $last_name;
 return $register;
 } else {
  if(empty($first_name)){
  $errors[] = 'You forgot to enter your
	    username.';
  return false;
  }elseif(empty($last_name)){
  $errors[] = 'You forgot to enter your
	    lastname.';
  return false;}
 }
}

[Muddy_Funster]

you're still returning false, you need to return the errors array, then change your logic on the main script to check what is returned, probably using is_array() and is_object() to see if it's the errors that are returned or the $register object

[designanddev]

ive tried this...

 

public static function Reg($first_name, $last_name) {
   if(!empty($first_name) && !empty($last_name)) {
  $register = new User();
 $register->first_name = $first_name;
 $register->last_name = $last_name;
 return $register;
 } elseif(empty($first_name)){
  $errors[] = 'You forgot to enter your
	    username.';
  return false;
  }elseif(empty($last_name)){
  $errors[] = 'You forgot to enter your
	    lastname.';
  return false;}
 }

 

 

and my submit is now

 

  if(isset($_POST['submit'])) {
  $first_name = trim($_POST['first_name']);
  $last_name = trim($_POST['last_name']);

  $new_user = User::Reg($first_name, $last_name);
  if($new_user && $new_user->save()) {
  // comment saved
  // No message needed; seeing the comment is proof enough.
 // Important!  You could just let the page render from here.
 // But then if the page is reloaded, the form will try
  // to resubmit the comment. So redirect instead:
 redirect_to("photo.php?id={$photo->id}");

 } else {
  // Failed
 $message = $new_user->errors;
 }
} else {
 $first_name = "";
 $last_name = "";
}

[designanddev]

you're still returning false, you need to return the errors array, then change your logic on the main script to check what is returned, probably using is_array() and is_object() to see if it's the errors that are returned or the $register object

 

im not quiet sure what you mean

[Muddy_Funster]

ok, take a look at the difference here :

public static function Reg($first_name, $last_name) {
 if(!empty($first_name) && !empty($last_name)) {
$register = new User();
	 $register->first_name = $first_name;
	 $register->last_name = $last_name;
	 return $register;
}
else{
if(empty($first_name)){
$errors[] = 'You forgot to enter your
				 username.';

}
if(empty($last_name)){
$errors[] = 'You forgot to enter your
				 lastname.';
}
return $errors;
}
}

//------------------------------------------------------------------------------
//##############################################################################
//------------------------------------------------------------------------------

if(isset($_POST['submit'])) {
$first_name = trim($_POST['first_name']);
$last_name = trim($_POST['last_name']);

$new_user = User::Reg($first_name, $last_name);
if(!is_array($new_user) && is_object($new_user) && $new_user->save()) {
// comment saved
// No message needed; seeing the comment is proof enough.
	 // Important! You could just let the page render from here.
	 // But then if the page is reloaded, the form will try
// to resubmit the comment. So redirect instead:
	 redirect_to("photo.php?id={$photo->id}");

} elseif(!is_object($new_user) && is_array($new_user)) {
// Failed
foreach($new_user as $errorMessage){
	 echo $errorMessage."<br />";
}
} else {
$first_name = "";
$last_name = "";
}

 

Edt : added } to code

Edited December 31, 2012 by Muddy_Funster

[designanddev]

Thank you i see but was there any need for the is_object function and is_array function?

also i have my error message function that i used to place the errors anywhere on my page

now how will i but this back into my function which is..

 

$message = join("<br />", $newuser->errors);
<?php echo output_message($message); ?>

 

i hope you understand

Thanks for your help

[Muddy_Funster]

the is_array and is_object are there to check on the datatype that is returned, so you can react accordingly.

 

to pass it into the the errors method, rather than making the errors array in the Reg method, simply point the message to your existing errors method using

$this->errors("You forgot to enter yuopr user name");

 

I'm making a few assumptions here as you havn't posted anything about an errors method or any other part of your class outside the Reg method.

[designanddev]

Ok heres an example of error messages i have been using....

 

 

Submit form

if(isset($_POST['submit'])){
$photo = new Photograph();
$photo->caption = $_POST['caption'];
$photo->category_id = $_POST['category'];
$photo->created = strftime("%Y-%m-%d %H:%M:%S", time());
$photo->attach_file($_FILES['file_upload']);
$photo->admin_id = $session->user_name;
//Success
if($photo->save()){
$session->message("Photograph uploaded successfully.");
redirect_to('list_photos.php');
}else{
//Failure
$message = join("<br />", $photo->errors);
}
}

 

 

Function...

public function attach_file($file) {
 // Perform error checking on the form parameters
 if(!$file || empty($file) || !is_array($file)) {
   // error: nothing uploaded or wrong argument usage
   $this->errors[] = "No file was uploaded.";
   return false;
 } elseif($file['error'] != 0) {
   // error: report what PHP says went wrong
   $this->errors[] = $this->upload_errors[$file['error']];
   return false;
 } else {
  // Set object attributes to the form parameters.
   $this->temp_path  = $file['tmp_name'];
   $this->filename   = basename($file['name']);
   $this->type	   = $file['type'];
   $this->size	   = $file['size'];
  // Don't worry about saving anything to the database yet.
  return true;
 }
}

 public function save() {
 // A new record won't have an id yet.
 if(isset($this->id)) {
  // Really just to update the caption
  $this->update();
 } else {
  // Make sure there are no errors

  // Can't save if there are pre-existing errors
   if(!empty($this->errors)) { return false; }

  // Make sure the caption is not too long for the DB
   if(strlen($this->caption) > 255) {
   $this->errors[] = "The caption can only be 255 characters long.";
   return false;
  }

   if(empty($this->caption)) {
   $this->errors[] = "please add a caption.";
   return false;
  }

   // Can't save without filename and temp location
   if(empty($this->filename) || empty($this->temp_path)) {
  $this->errors[] = "The file location was not available.";
  return false;
   }

  // Determine the target_path
   $target_path = "/home/designanddev.co.uk/public_html/blog/".$this->upload_dir."/".$this->filename;

   // Make sure a file doesn't already exist in the target location
   if(file_exists($target_path)) {
  $this->errors[] = "The file {$this->filename} already exists.";
  return false;
   }

  // Attempt to move the file
  if(move_uploaded_file($this->temp_path, $target_path)) {
 // Success
   // Save a corresponding entry to the database
   if($this->create()) {
 // We are done with temp_path, the file isn't there anymore
 unset($this->temp_path);
 return true;
   }
  } else {
   // File was not moved.
  $this->errors[] = "The file upload failed, possibly due to incorrect permissions on the upload folder.";
  return false;
  }
 }
}

 

And this function here allows me to locate the error messages anywhere within my script

<?php echo output_message($message); ?>

 

Hope this is understandable

[Muddy_Funster]

am I right in thinking that your are pretty new to OOP? Anyway, the only thing that's really going to help is to see the full class that youe are using.

[designanddev]

am I right in thinking that your are pretty new to OOP? Anyway, the only thing that's really going to help is to see the full class that youe are using.

 

Ok i have been displaying my errors without using foreach loops my code was very clean after i tried to add more feature from after the tutorial, i want a way to display my errors like the example i photo example i posted earlier, i do not get the difference, im trying to use the same technique when displaying errors

[Muddy_Funster]

then you need to use the same technique when assigning errors. I don't even know if your photo upload is the same class as the register user one, I don't know what format you have used to define the errors property of the class, in short, other than what you show me, I don't know your code. Without seeing the whole class I can't help.

[designanddev]

This is the whole user class

 

<?php
require_once("database.php");
class User extends DatabaseObject{
protected static $table_name = "users";
protected static $db_fields = array('id', 'username', 'password', 'first_name', 'last_name', 'email');
public $id;
public $username;
public $password;
public $first_name;
public $last_name;
public $email;
public $errors = array(); // Initialize an error array.

// Check for an email first name:
public static function Reg($first_name, $last_name) {
 if(!empty($first_name) && !empty($last_name)) {
  $register = new User();
  $register->first_name = $first_name;
  $register->last_name = $last_name;
  return $register;
 }
else{
if(empty($first_name)){
  $errors[] = 'You forgot to enter your
  username.';

}
if(empty($last_name)){
  $errors[] = 'You forgot to enter your
  lastname.';
}
return $errors;
}}


public static function Authenticate($username="", $password=""){
global $database;
$username = $database->escape($username);
$password = $database->escape($password);
$sql = "SELECT * FROM users ";
$sql .= "WHERE username = '{$username}' ";
$sql .= "AND password = '{$password}' ";
$sql .= "LIMIT 1";
$result_array = self::find_by_sql($sql);
return !empty($result_array)? array_shift($result_array): false;
}
public function fullname(){
if(isset($this->first_name) && isset($this->last_name)){
return "first name: ".$this->first_name."</br> last name: ".$this->last_name;
}else{
return "";
}
}

//Common Database Methods
public static function find_all(){
global $database;
$result_set = self::find_by_sql("SELECT * FROM ".self::$table_name);
return $result_set;
}
public static function find_id($id=0){
global $database;
$result_array = self::find_by_sql("SELECT * FROM ".self::$table_name." WHERE ID = {$id}");
return !empty($result_array) ? array_shift($result_array) : false;
}
public static function find_by_sql($sql=""){
global $database;
$result_set = $database->query($sql);
$object_array = array();
while($row = $database->mysql_fetch($result_set)){
$object_array[] = self::instantiate($row);
}
return $object_array;
}

private function instantiate($record){

$object = new self;
//$object->username   = $record['username'];
//$object->password   = $record['password'];
//$object->first_name = $record['first_name'];
//$object->last_name  = $record['last_name'];
foreach($record as $attribute => $value){
if($object->has_attribute($attribute)){
$object->$attribute = $value;
}
}
return $object;
}

private function has_attribute($attribute){
$object_vars = $this->attributes();
return array_key_exists($attribute, $object_vars);
}
protected function attributes() {
 // return an array of attribute names and their values
  $attributes = array();
  foreach(self::$db_fields as $field) {
 if(property_exists($this, $field)) {
   $attributes[$field] = $this->$field;
 }
  }
  return $attributes;
}
protected function sanitized_attributes() {
  global $database;
  $clean_attributes = array();
  // sanitize the values before submitting
  // Note: does not alter the actual value of each attribute
  foreach($this->attributes() as $key => $value){
 $clean_attributes[$key] = $database->escape($value);
  }
  return $clean_attributes;
}
public function save() {
  // A new record won't have an id yet.
  return isset($this->id) ? $this->update() : $this->create();
}
public function create() {
 global $database;
 // Don't forget your SQL syntax and good habits:
 // - INSERT INTO table (key, key) VALUES ('value', 'value')
 // - single-quotes around all values
 // - escape all values to prevent SQL injection
 $attributes = $this->sanitized_attributes();
  $sql = "INSERT INTO ".self::$table_name." (";
 $sql .= join(", ", array_keys($attributes));
  $sql .= ") VALUES ('";
 $sql .= join("', '", array_values($attributes));
 $sql .= "')";
  if($database->query($sql)) {
 $this->id = $database->insert_id();
 return true;
  } else {
 return false;
  }
}

public function update() {
  global $database;
 // Don't forget your SQL syntax and good habits:
 // - UPDATE table SET key='value', key='value' WHERE condition
 // - single-quotes around all values
 // - escape all values to prevent SQL injection
 $attributes = $this->sanitized_attributes();
 $attribute_pairs = array();
 foreach($attributes as $key => $value) {
   $attribute_pairs[] = "{$key}='{$value}'";
 }
 $sql = "UPDATE ".self::$table_name." SET ";
 $sql .= join(", ", $attribute_pairs);
 $sql .= " WHERE id=". $database->escape($this->id);
  $database->query($sql);
  return ($database->affected_rows() == 1) ? true : false;
}
public function delete(){
global $database;
 // Don't forget your SQL syntax and good habits:
 // - DELETE FROM table WHERE condition LIMIT 1
 // - escape all values to prevent SQL injection
 // - use LIMIT 1
  $sql = "DELETE FROM ".self::$table_name;
  $sql .= " WHERE id=". $database->escape($this->id);
  $sql .= " LIMIT 1";
  $database->query($sql);
  return ($database->affected_rows() == 1) ? true : false;

 // NB: After deleting, the instance of User still
 // exists, even though the database entry does not.
 // This can be useful, as in:
 //   echo $user->first_name . " was deleted";
 // but, for example, we can't call $user->update()
 // after calling $user->delete().
}

}//END OF USER
?>

[Muddy_Funster]

yeah, so all you need to do is change the line

errors[] = "you forgot..."

to

$this->errors[] = "your forgot..."

just as you have it in the save() method of your photo manager class. unless you make a whole new class extension template you will need to address each error property sepporately. the $errors in your User class is not the same $errors that is in your photo manager class, although it can be accessed in the same way as long as it's assigned in the same way.

[designanddev]

yeah, so all you need to do is change the line

errors[] = "you forgot..."

to

$this->errors[] = "your forgot..."

just as you have it in the save() method of your photo manager class. unless you make a whole new class extension template you will need to address each error property sepporately. the $errors in your User class is not the same $errors that is in your photo manager class, although it can be accessed in the same way as long as it's assigned in the same way.

 

when i use $this is breaks and comes up with an error message

[designanddev]

what about the return what do i do, should i return false like i current had done first time round

[Muddy_Funster]

yeah, sorry. as you are populating the class level parameter you don't need any further return from those checks

[designanddev]

i keep geting this error...

 

Fatal error: Using $this when not in object context in /home/designanddev.co.uk/public_html/blog/includes/users.php on line 31

[Muddy_Funster]

try using

self::errors[] = "you forgot...";

incase it's something funky about the static method call, failing that you can always try

User::errors[] = "you forgot...";

[designanddev]

try using

self::errors[] = "you forgot...";

incase it's something funky about the static method call, failing that you can always try

User::errors[] = "you forgot...";

 

Now ive got this error..

 

Fatal error: Access to undeclared static property: User::$errors

[designanddev]

Now ive got this error..

 

Fatal error: Access to undeclared static property: User::$errors

 

if(empty($first_name)){
  User::$errors[] = 'You forgot to enter your
  username.';
  return false;

}
if(empty($last_name)){
  User::$errors[] = 'You forgot to enter your
  lastname.';
  return false;
}

[Christian F.]

I started to clean up your code a bit, and adding comments to highlight problematic areas. However, unfortunately your code is such a mess that it would be easier for me to just rewrite it from scratch. :\

You need to study the design philosophies behind object-oriented programming a bit more, I think, in order to grasp how to structure your code properly. This will allow you to cut down on a lot of unnecessary complexity, and thus make a lot of your troubles simply vanish. For this I would actually recommend the following video by Anthony Ferrara, even though it goes through things a bit quickly. It is a very good example on how to properly structure your OOP code, as he explains the reasoning behind his choices.

 

For a quick list of the main problems in your code:

• You should never, ever use the global keyword in OOP. In fact, it should almost never be necessary to use it.
  
• The user class should not be an extension of the Database class, but it should use a database object. This can be solved either by using dependency injection (using) or by passing an database object as a parameter to the constructor.
  
• Having an object create another instance of itself, when adding users is also wrong as this completely breaks the "single responsibility" principle. A new object should only be created by a factory class, or by controller which created the object in the first place.
  It makes no sense for a user to create a new and completely unrelated user, after all. That's like having a car creating another car, for your neighbour.
  
• A class should either be static (abstract) or instantiated, never both. If you find yourself mixing static method calls (self::) and object-related ($this->) then chances are that you've just overcomplicated things. There are some honourable mentions, but unless you're 100% certain of what you're doing you should never encounter these.
  
• You've also mixed a form of dependency injection (using global), static references, and class inheritance for the database connection. Which, in turn, adds a lot of unnecessary complexity and uncertainty to your code. Pick one, and stick with it.
  

 

As noted above I really think that you'll find the video useful, and you should also take some time to read up on the different design principles surrounding OOP. Especially the single responsibility principle. Once you've done that, and taken the time to sit down and plan out your classes before writing them, you should find that this task has become substantially easier to accomplish.

[designanddev]

Great il take a look.

[designanddev]

ok il post something real simple... and all i want to do is access these errors on the other side, outside of the class

 

public function Reg() {
if(empty($first_name)){
  $this->errors[] = 'You forgot to enter your
  username.';
  return false;
}
if(empty($last_name)){
  $this->errors[] = 'You forgot to enter your
  lastname.';
  return false;
}

elseif(empty($this->errors)){
echo "hello";
}
}