asked on Form validation using functions
I am trying to learn how to use functions to make my code neater by not having loads of php in amongst the html. It seems to be working okay so far for making queries and inserting/editing/delete data in the database. But I am having an issue with validation.
Before the function I had:
As soon as I put that into a function and call it on the page I want it on, I get a notice:
Undefined variable: error in /Applications/MAMP/htdocs/
Line 54 is:
This used to work just fine before using the function to call this code.
I defined the variable in the function and am calling the function before the $error.$success code so why is it saying the variable is undefined?
Here is the function:
Perhaps I should rather exclude the validation from the function, perform it on the page and if it passes, then only call the function?
Before the function I had:
$error = "";
$success ="";
if(isset($_POST['submit'])) {
if (empty($_POST['cat_title'])){
$error .= "Category cannot be empty";
} else {
$cat_title = $link->real_escape_string($_POST['cat_title']);
$sql = "INSERT INTO `categories`(cat_title) VALUES('$cat_title')";
if ($result = $link->query($sql) == TRUE) {
$success .="record added";
As soon as I put that into a function and call it on the page I want it on, I get a notice:
Undefined variable: error in /Applications/MAMP/htdocs/
Line 54 is:
<div><?php echo $error.$success;?></div>
This used to work just fine before using the function to call this code.
I defined the variable in the function and am calling the function before the $error.$success code so why is it saying the variable is undefined?
Here is the function:
function insert_categories(){
global $link;
$error = "";
$success ="";
if(isset($_POST['submit'])) {
if (empty($_POST['cat_title'])){
$error .= "Category cannot be empty";
} else {
$cat_title = $link->real_escape_string($_POST['cat_title']);
$sql = "INSERT INTO `categories`(cat_title) VALUES('$cat_title')";
if ($result = $link->query($sql) == TRUE) {
$success .="record added";
Perhaps I should rather exclude the validation from the function, perform it on the page and if it passes, then only call the function?
PHP
Last Comment
Marco Gasi
Functions work by passing and returning data. Since '$success' is defined inside the function and is not 'global', it is not available to the code outside the function. The only reason that $_POST works is because it is declared a 'super-global' in PHP.
This page http://php.net/manual/en/functions.arguments.php shows how to pass and return arguments in PHP functions. Normally the last line in your function would be 'return $success;' which passes it back to the line that called the function.
This page http://php.net/manual/en/functions.arguments.php shows how to pass and return arguments in PHP functions. Normally the last line in your function would be 'return $success;' which passes it back to the line that called the function.
Can you show us your full function - you have only posted some of it - which does not contain the error line. The error is saying that $error is not defined - the compiler is always right so you need to show us where this line is
Aside: Don't use global - for too many reasons to mention. Either put your function inside a class and make the $link a property or pass the $link value into the function as a parameter.
<div><?php echo $error.$success;?></div>
Aside: Don't use global - for too many reasons to mention. Either put your function inside a class and make the $link a property or pass the $link value into the function as a parameter.
ASKER CERTIFIED SOLUTION
THIS SOLUTION ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
Check this, and please ask questions about it. It's one of the most important things you can learn:
http://php.net/manual/en/language.variables.scope.php
See also:
http://php.net/language.functions
In this particular question-and-answer dialog, please learn that global is an antipractice. PHP is a very old language, and a lot of the examples you will find online are old. They are outdated and create security holes. We did things differently a decade ago, and through experience we have come to learn that we were doing it wrong. Unfortunately, all those old PHP examples are still hanging around on the internet, just waiting to poison the minds of new developers. They do not rot away.
The opposite of the evil and stupid global is dependency injection. It's a little hard to explain, but I've made an effort here:
https://www.experts-exchange.com/articles/19999/PHP-Design-Avoiding-Globals-with-Dependency-Injection.html
https://www.experts-exchange.com/articles/18210/Software-Design-Dependencies.html
http://php.net/manual/en/language.variables.scope.php
See also:
http://php.net/language.functions
In this particular question-and-answer dialog, please learn that global is an antipractice. PHP is a very old language, and a lot of the examples you will find online are old. They are outdated and create security holes. We did things differently a decade ago, and through experience we have come to learn that we were doing it wrong. Unfortunately, all those old PHP examples are still hanging around on the internet, just waiting to poison the minds of new developers. They do not rot away.
The opposite of the evil and stupid global is dependency injection. It's a little hard to explain, but I've made an effort here:
https://www.experts-exchange.com/articles/19999/PHP-Design-Avoiding-Globals-with-Dependency-Injection.html
https://www.experts-exchange.com/articles/18210/Software-Design-Dependencies.html
ASKER
Wow, so many good answers. Sometimes giving points is harder than trying to understand the solutions!
ASKER
I am going to try work my way down the answers.
@Julian, that was the whole function but after what you said I did change it to include the $error.$success echo in the function itself like this:
And then I just placed the call to the function in the html where I wanted the message to display.
The tutorial I was following used global. But it sounds like that is a bad idea!
@Julian, that was the whole function but after what you said I did change it to include the $error.$success echo in the function itself like this:
<?php
function insert_categories(){
global $link;
$error = "";
$success ="";
if(isset($_POST['submit'])) {
if (empty($_POST['cat_title'])){
$error .= "Category cannot be empty";
} else {
$cat_title = $link->real_escape_string($_POST['cat_title']);
$sql = "INSERT INTO `categories`(cat_title) VALUES('$cat_title')";
if ($result = $link->query($sql) == TRUE) {
$success .="record added";
}
}
echo $error.$success;
}
}
?>
And then I just placed the call to the function in the html where I wanted the message to display.
<div class="form-group">
<input type="submit" class="btn btn-primary "name="submit" value="Add Category">
<div><?php insert_categories();?></div>
</div>
The tutorial I was following used global. But it sounds like that is a bad idea!
SOLUTION
THIS SOLUTION ONLY AVAILABLE TO MEMBERS.
View this solution by signing up for a free trial.
Members can start a 7-Day free trial and enjoy unlimited access to the platform.
ASKER
Sorry, you are right Julian. The reason I had 2 i.e.: $error and $success is that I wanted to have different classes for them so that the error had a red background and the success had a green background. That was my thinking behind having the 2 messages.
Yes, global is a bad idea. There are many old, bad ideas on the internet. The most pernicious ones frustrate automated testing. Global declarations have been shown too be one of the big mistakes in software design. If you want another view of how bad this problem can be, look at the way JavaScript handles undeclared variables!
Sidebar note:
$cat_title = empty($_POST['cat_title'])
? $link->real_escape_string($_POST['cat_title'])
: false;
$cat_title = !empty($_POST['cat_title'])
? $link->real_escape_string($_POST['cat_title'])
: false;
ASKER
Thanks for everyones patience.
I am trying to avoid the usage of global as suggested but doing this gives me an error:
Warning: Missing argument 1 for insert_categories(), called in /Applications/MAMP/htdocs/
line 53 is :
Line 3 is:
I am trying to avoid the usage of global as suggested but doing this gives me an error:
function insert_categories($link){
Warning: Missing argument 1 for insert_categories(), called in /Applications/MAMP/htdocs/
line 53 is :
<div><?php echo insert_categories();?></div>
Line 3 is:
function insert_categories($link){
Yup, or
$cat_title = empty($_POST['cat_title'])
? false;
: $link->real_escape_string($_POST['cat_title'])
ASKER
Oh, never mind. I forgot to do this:
<div><?php echo insert_categories($link);?></div>
You need to pass $link to the function when you call it
<div><?php echo insert_categories($link);?></div>
ASKER
Was my reasoning for having both $error and $success okay or am I still wrong?
$error .= '<div class="alert alert-danger" role="alert">Category cannot be empty</div>';
$success = '<div class="alert alert-success" role="alert">Record added</div>';
If you are going that route you need two bits of information
The result of the query and the message. A function can only return one value so you either need to return an object / array or you need to change your logic.
The quick fix is to just change the function to create the entire <div> complete with alert classes - not really good design but it will work in the context of what you are trying to do.
Something like this
The result of the query and the message. A function can only return one value so you either need to return an object / array or you need to change your logic.
The quick fix is to just change the function to create the entire <div> complete with alert classes - not really good design but it will work in the context of what you are trying to do.
Something like this
<?php
function insert_categories($link)
{
$response = "";
$cat_title = empty($_POST['cat_title'])
? $link->real_escape_string($_POST['cat_title'])
: false;
if ($cat_title) {
$sql = "INSERT INTO `categories`(cat_title) VALUES('{$cat_title}')";
$result = $link->query($sql);
if ($result) {
$response = '<div class="alert alert-success">Record added</div>';
}
//EDIT
else {
$response = '<div class="alert alert-danger">Insert failed with error ' . $link->error . "</div>";
}
}
else {
$response = '<div class="alert alert-danger">Category cannot be empty</div>';
}
return $response;
}
// This gets called at the start of your script
$result = $insert_categories($link);
?>
...
<div class="form-group">
<input type="submit" class="btn btn-primary "name="submit" value="Add Category">
<?php echo $result?>
</div>
ASKER
Thanks Julian. I tried your code and the error message displays instantly before even submitting the form.
ASKER
Sorry, one last thing. Why is it not good to echo out the errors like I had them with the div tags i.e.:
$response = '<div class="alert alert-danger">Category cannot be empty</div>';
It looks like you totally ignored my comment https://www.experts-exchange.com/questions/28968945/Form-validation-using-functions.html?anchorAnswerId=41792540#a41792540
Or am I missing something?
Or am I missing something?
ASKER
Sorry Marco, you are correct. I forgot to allocate some points to you. How can I change that?
Because you are mixing your status messages with the presentation. If you want to change the styling of the message you have to go into the function and change it there.
Ideally you want your error message to be just the text.
One option is to assume that an empty message means success and have a default message so you could do this
Ideally you want your error message to be just the text.
One option is to assume that an empty message means success and have a default message so you could do this
$error= insert_category($link);
// Assume the worst
$class = 'alert-danger';
if (empty($error)) {
$class = 'alert-success';
$error = 'Record Added';
}
...
<div class="alert <?php echo $class;?>"><?php echo $error;?></div>
$class = 'hidden';
if ($_POST) {
// above code with call to insert_category here
}
// If no POST we drop through and class hidden kicks in and hides the status <div>
Ok, don't worry. I just wanted to understand what happened since the accepted solution presented a code substantially identical to mine.
On to the next :)
On to the next :)
ASKER
I chose Julian's as the best because he showed me how to not use global. Apologies, I will try allocate better next time, it's not that easy! ;)
ASKER
@ Julian. That makes sense in your example but I don't know how to implement into existing function?
function insert_categories($link){
$response = "";
$class = 'alert-success';
if (isset($_POST['cat_title'])) {
if (!empty($_POST['cat_title'])){
$cat_title = $link->real_escape_string($_POST['cat_title']);
$sql = "INSERT INTO `categories`(cat_title) VALUES('$cat_title')";
if ($result = $link->query($sql) == TRUE) {
//$response = '<div class="alert alert-success" role="alert">Record added</div>';
$response = "Record added";
}
} else {
$class = 'alert-danger';
//$response = '<div class="alert alert-danger" role="alert">Category cannot be empty</div>';
$response = "Category cannot be empty";
}
return $response;
}
}
<div><?php echo insert_categories($link);?></div>
ASKER
I hope that's fair. I found both very helpful for different reasons.
Thank you Black Sulfur. It wan't necessary (as I said) but thank you again.
https://www.experts-exchange.com/articles/11769/And-by-the-way-I-am-New-to-PHP.html