eldan88 Posted March 11, 2014 Share Posted March 11, 2014 Hey Guys. I am trying to learn how to use methods to validate forms. Below I have a simple form with just two fields and a method called validation. Right now this is a very basic function. I am trying to get this to work first before adding more functionality to it. When I tested the code in the browser I got to error messages: Warning: Invalid argument supplied for foreach() in /Applications/XAMPP/xamppfiles/htdocs/sandbox/form_validation.php on line 7Notice: Undefined index: submit in /Applications/XAMPP/xamppfiles/htdocs/sandbox/form.php on line 3 I'm not exactly sure on how to fix this... Below is the code I wrote. Any help would be really appreciated! form_validation.php file <?php class validation { public function validation($form_fields="") { $errors = array(); foreach ($form_fields as $form_field) { if(empty($_POST[$form_field])) { $errors[] = "Form Fields are empty"; }// end of if return $errors; }// end of foreach } } $form_validation = new validation(); ?> form.php <?php include("form_validation.php"); if($_POST['submit']) { $name = $_POST['name']; $last_name = $_POST['last_name']; $fields = array($name, $last_name ); $form_validation->validation($fields); echo $errors; } ?> <!doctype html> <html> <body> <form action="#" method="POST"> <input type="text" name="name" id="name"> <input type="text" name="last_name" id="last_name"> <input type="button" value="submit" name"submit"> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
.josh Posted March 11, 2014 Share Posted March 11, 2014 The most immediate issue you have is that you are creating a new validation object, and you have your form validation code inside the constructor. If you name a method the same name as the class, that becomes the constructor that gets executed when you create an object of that class. This is the old php4 style of making a constructor; you shouldn't really be doing it that way unless you are using php4 (in which case, update your php version 5+ and use the new style syntax). But regardless, you shouldn't be putting your form validation in the constructor, because when you make that new object $form_validation it's going to run the code. And you are making it before you even fill out and submit the form, so nothing is getting passed to it, which is making $form_fields default to an empty string, which is making your foreach loop attempt to iterate over a string instead of an array. So, basically you need to change your code to call your validation when the form is submitted. Which you already do, except for the fact that your method is your constructor. So if you absolutely want to stick with the php4 syntax, rename your validation method to something like validate() or something else other than the class name, and leave the rest of the code as-is (except for below, read on). Also, as far as your actual validation is concerned.. there is one outright error, and another issue that's not so much an issue as a matter of better coding. Firstly, you pass the form values to your validation function, and yet do this: $_POST[$form_field]. This is going to make the condition look for an index of the form value, not the form element name. E.g. if I put "Crayon" as my name and "Violent" as my last_name, it's going to look for $_POST['Crayon'] and $_POST['Violent']. Which brings me to my 2nd point: You shouldn't be looking at $_POST in your function in the first place. You passed the form values to the function, so why are you looking at $_POST? You should just be doing if (empty($form_field)) {..}. Quote Link to comment Share on other sites More sharing options...
jairathnem Posted March 11, 2014 Share Posted March 11, 2014 <form action="#" method="POST"> action is blank here. Quote Link to comment Share on other sites More sharing options...
eldan88 Posted March 11, 2014 Author Share Posted March 11, 2014 Hey Josh. Thank you for the useful post. I forgot the php4 does a construct if the method and the class name is the same. I just gave them the same named unintentionally. I changed the code as you mentioned, but still can't seem to see any erros when I submit a blank form. Below is the modified form. Thank you very much for your help... form_validation.php <?php class validation { public function validate_fields($form_fields="") { $errors = array(); foreach ($form_fields as $form_field) { if(empty($form_field)) { $errors[] = "Form Fields are empty"; }// end of if return $errors; }// end of foreach } } $form_validation = new validation(); ?> form.php <?php include("form_validation.php"); if(isset($_POST['submit'])) { $name = $_POST['name']; $last_name = $_POST['last_name']; $fields = array($name, $last_name ); $form_validation->validation($fields); echo $errors; } ?> <!doctype html> <html> <body> <form action="#" method="POST"> <input type="text" name="name" id="name"> <input type="text" name="last_name" id="last_name"> <input type="submit" value="submit" name"submit"> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
Solution .josh Posted March 11, 2014 Solution Share Posted March 11, 2014 Okay, there are a number of issues wrong still. You need to work on your debugging skills. Start at the beginning. - You have a form, and you click submit. What is the first thing your script does? It checks to see if $_POST['submit'] is set. So, put print_r($_POST); above that condition and see what you are getting. Do you see that it is being set? From this, you can see that there is no $_POST['submit'] in the array dump. - Look at your form. Did you typo something somewhere? Look closely, you can see that you did name"submit" when it should be name="submit" - Now that that is sorted, what does your script do next? It assigns some posted values to some variables and then calls your method. Is there a typo in the $_POST indexes? Is there a typo in the variable names? Is there a typo in the method or argument passed? A typo in the method name you call?Yes. You made the change to your method name, but didn't change the name where you are calling it. - Now examine your method. Are you sure it's receiving the values (even if they are empty)? echo out something in your foreach like "foreach called". Are you sure the condition is working as intended? echo out "condition triggered". Are you sure $errors is being set with something? echo out $errors (before you return it). Is return even in the right place? This isn't so much an error but a matter of "sense": technically this part is right in that it will return an error on first error encountered. However, your implied intention vs. your code don't seem to match up. You make $errors an array as if you want to return multiple errors, one for each field. But you have a generic message in it and return on first error. You need to pick one style or the other. If you want to just return a generic message on first error, ditch making it an array. But if you want a specific error message for each element, make the message specific, e.g. "{$field} is empty". In order to do that, you will also need to pass the element names (the post keys) to your function, not just the values). - Now go back to where you called the method. You returned a value from your method. Are you doing anything with it? No. You just called the function and returned a value but you didn't assign the returned value to anything. Instead, you just echo'd out $errors. That's not how returned values and variables within functions work. There is scope to variables within functions. The code that called your function doesn't know anything about the variable you set in the function. Therefore you need to assign the function call to a variable, or else echo out the function call: $errors = $form_validation->validate_fields($fields); echo $errors; // or echo $form_validation->validate_fields($fields); - Is the error now outputting when something is empty? Maybe? If you had changed $errors to just be a string instead of array within your validation method, you should see the error echo'd now. But as-is, you should be seeing "Array". That's because you assigned it as an array, not a string, and that's what you get when you try to echo an array. So now we're back to the generic message vs. specific message debate. If you want to just output a generic message on any error, go back and change it to be a string instead of array (remove the []). If you want a list of error messages to be returned, you need to check if the array has error messages and output them accordingly, e.g. : $errors = $form_validation->validate_fields($fields); // example: implode them to a string to echo, where each message is on a separate line. if (count($errors)>0) echo implode("<br/>",$errors); Okay, so this was a lot of typing, so I hope you take the time to use this to learn how to fish for yourself (debug), instead of just fix the problem and then ask for more fish. If you have an issue, walk through your code one line at a time. Figure out what you are expecting and output what you can to validate that you get what you are expecting. Also, while developing, turn on all error reporting. There are a number of issues that would have displayed errors as clues if your error reporting had been turned on properly. Put this at the top of your code: ini_set('display_startup_errors',1); ini_set('display_errors',1); error_reporting(-1); Quote Link to comment Share on other sites More sharing options...
eldan88 Posted March 11, 2014 Author Share Posted March 11, 2014 Hey Josh. I just read through your whole post. First I want to say thank you for taking your time and making everything more clear to me. Espically by putting great emphasis on debugging my code since it saves a lot of time for developers. Your post made a lot more sense to. I guess I was rushing instead of taking my time and going through the code line by line, thoroughly checking for typos, and debugging my code. I am very familiar on how to debug code... I just overlooked it. I will keep in mind on what you said from now on, and just take my time before rushing into asking questions. I will not re-check my whole code, and get this to work. As for the general message vs the specific message debate. I do want to show specific messages shown for the errors, but I will first try out with the generic message, then go move on with the specific message. I might have a question later on about showing specific error messages since it is my first time... But I think I will get it... Seems self explanatory Quote Link to comment Share on other sites More sharing options...
eldan88 Posted March 11, 2014 Author Share Posted March 11, 2014 P.S I went through everything again and not only is the form displaying error's its now displaying specific error messages.. Thank you!! form_validation.php <?php class validation { public function validate_fields($form_fields="") { $errors = array(); foreach ($form_fields as $key=>$value) { //if(empty($value)) { $errors[] = str_replace("_", " ", $key) . " Is empty"; //}// end of if //return $errors; }// end of foreach return $errors; }// end of function }// end of class $form_validation = new validation(); ?> form.php <?php include("form_validation.php"); if(isset($_POST['submit'])) { //$name = $_POST['name']; //$last_name = $_POST['last_name']; // $fields = array($name, $last_name ); $_POST; unset($_POST['submit']); $errors = $form_validation->validate_fields($_POST); if (count($errors)>0) echo implode("<br/>",$errors); } ?> <!doctype html> <html> <head></head> <body> <form action="#" method="POST"> <input type="text" name="Name" id="name"> <input type="text" name="last_name" id="last_name"> <input type="submit" value="submit" name="submit"> </form> </body> </html> Quote Link to comment Share on other sites More sharing options...
eldan88 Posted March 11, 2014 Author Share Posted March 11, 2014 <form action="#" method="POST"> action is blank here. Hey. The reason why it is blank because the form is submitting to itself. Quote Link to comment Share on other sites More sharing options...
jairathnem Posted March 12, 2014 Share Posted March 12, 2014 Hey. The reason why it is blank because the form is submitting to itself. wow. I didn't know that would work! thanks! Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.