marking the form errors

[jimfog]

I have a form and I want to mark(after the user submits it) the errors in it-something very typical in the web today. SO far, I have code the validating functions and the scheme goes like that:

function profile_check($required,$services,$pricelist){  	 $correctdata=true;	  if(!(val_phone($required['phone'])))		  {$correctdata=false;		  echo '<h2>You have the following errors.</h2>			  <br>You did not enter the correct phone format<br>';}       	  if(!(empty($services['service1']))||!(empty($pricelist['price1'])))			 { val_evr_else($services['service1'],$pricelist['price1']); } return $correctdata;       }

I have also prepared the error message(for example above you see the error message for incorrect phone submission) Here is part of the form:

<div id="form">							    <form   action="profile.php" method="post">									  <div class="basics">										  <h2>The Basic data</h2>										    <label class="label" for="phone">Required field</label> <input <?php if( $correctdata==false){echo 'id="phone_error"';}else{echo 'id="phone"';} ?>size="40" placeholder="Τηλέφωνο" type="text" name="phone"><br>										    <label  class="label" for="address">Required field</label><input id="address" size="40" placeholder="Διεύθυνση" type="text" name="address"><br>										    <label  class="label" for="city">Required field</label><input size="40" placeholder="Πόλη" type="text" name="city"><br>										    <label  class="label" for="municipality">Required field</label><input size="40" placeholder="Νομός" type="text" name="municipality"><br>										  </div>

Ignore the Greek words.You could say that I do something like that:

<label class="label" for="phone">Required Field</label><input <?php if( $correctdata==false){echo 'id="phone_error"';}else{echo 'id="phone"';} ?>size="40" placeholder="Τηλέφωνο" type="text" name="phone"><br>

Yes the above is only for incorrect phone submission-how am I going to target the other inputs as well.As I see it profile_check() must return multiple variables which would be false when the validating functions return false and use these to echo the corresponding classes to the form, but I do not know if this is the way to go.

[jeffman]

I would suggest two things. 1. Write the HTML and text for each possible error message. Put in your document. Put each one in a unique container with the CSS display set to none. When PHP detects that an error message should be displayed, all you have to do is echo a new classname that changes the display property of the container. Do NOT use PHP to echo the error messages individually. 2. Don't return multiple variables. Return an array. Set each array element every time you run a test. Then you can run tests like this:

<div class="message<?php if ($validationErrors['phone'] ) {echo ' visible';} ?>">Correct telephone number required</div>

Edited January 23, 2013 by Deirdre's Dad

[jimfog]

Before going implementing what you suggest I must note the code I have created already and in which case I do not know how easy or difficult is to implement what you are saying.More specifically this is the way I validate some fields of the form-not shown above:

$validate = array(array('formfield'=>$pricelist,'validate'=>val_prices($pricelist),'error'=> wrong price format' ),array('formfield'=>$services,'validate'=> val_services($services),'error'=>wrong service format',])		 );   foreach ($validate as $value) {             if ($value['validate'] == false) {                 $correctdata=false;                 echo $value['error']. '<br>';             }         } function val_prices($pricelist) {         $price1tr=trim($pricelist);//λάθος εδώ...το trim δεν παίρνει array     if(is_numeric($price1tr))         return true;     else         return false;   }   function val_services($services) {     $service1tr=trim($services);       if(!(is_numeric($service1tr)))         return true;     else         return false;   } 

The form had 2 additional fields-price and service and the functions that validates them is, val_price and val_services-seen in the array above.Afterwards I just loop through the array in order to make the checks. Given the context above-do you think what you suggest can be easily integrated or I must write new validating code?

[jeffman]

You could maybe integrate my suggestion in your foreach loop. I don't know. BTW, look at your $validate array. Some characters are wrong/missing.

[jimfog]

I found a much better solution. I ditched completely the functions and a made a series of if statements(which do the checking).At the end of the script is the form.Just look at the code:

if (isset($_POST['submit']))			 {global $errors;			 if (filled_out_profile($_POST))				  { $phonepr= preg_replace(array('/^\+/', '/\D/','--'), array('', '',''), trim($_POST['phone']));				    if((strlen($phonepr) <5) || (strlen($phonepr) > 12))						  {global $errors;						    $errors.= "no proper phone format<br>";						    $errorphone=true;						     }				          			    if (!$errors) { 					  echo "Thank you!<br/><br/>"; 				    } else { 					    echo '<h2>You have the following errors</h2><div style="color: red">' . $errors . '<br/></div>'; 				    }		  				  }			 else		    {echo 'You did not filled all of the fields';}			 }	  			 		    ?>	   	  <div id="form">	    <form   action="profile.php" method="post">		 <div class="basics">		  <h2>Details</h2>		    <label class="label" for="phone">Required</label> <input id="<?php global $errorphone;  if($errorphone) {echo 'phone_error';} else{echo 'phone';} ?>" size="40" placeholder="phone" type="text" name="phone"><br>....other inputs here		

Tell me what you think-the functions just made things more complex-I have yet to learn whenI should use them.

[thescientist]

functions are quite simple and useful, I just think you're overthinking it too much. A function works best when it is in charge of doing one thing, and one thing only. A function typically takes some sort of input, and typically returns some output, which is the result of the function acting on the input. The params/return concept allows functions to be reusable and portable. (using things like the global keyword in functions, does not). Simply, if you need to do the same kind of "work" over and over, and each piece of work is fairly common in the types of input and output it expects, then that kind of work is a great candidate for a function. Often, these functions get collected together in one script, called a library, for inclusion in multiple projects, but this only works if functions are well written to be reusable and portable within their own scope. edit: also, putting code into functions means you can test it repeatedly with all sorts of input/output combinations, and make sure it runs well under all conditions. this keeps code clean, organized, and is easier to maintain.

Edited January 28, 2013 by thescientist

[jimfog]

. (using things like the global keyword in functions, does not).

This is something I do not understand. What if a variable MUST be seen outside the function too? Now...in regard to the code I demonstrate-what would you go for?The conditionals, or the functions way, as it was before? Working with the functions I encountered the following problem:I had to return more the 2 variables-something that complicated things. While now, with the conditionals, the variables are visible all over the script.Plus the fact that the form code is only once-at the end of the script-while before it was twice.

[thescientist]

This is something I do not understand. What if a variable MUST be seen outside the function too?

pass it as an argument into the function. The reason globals are evil is just for that reason, they are global, and that means any script, function, method, etc can get its hands all over it, making debugging and testing a pain. if there are "contracts" for data in, and data out, you can limit the scope of your efforts to just one function, or piece of code.

Now...in regard to the code I demonstrate-what would you go for?The conditionals, or the functions way, as it was before?\

I would go so far as to create a validation class (static, most likely) that has methods for all the common types of validation you may have in a form, like name, address, email, etc. Then to add validation support to any script, you just include that class and then simply call methods on it and pass your string to it, and get true/false back. Simple, clean, portable, maintainable, and even more so, testable.

Working with the functions I encountered the following problem:I had to return more the 2 variables-something that complicated things.

you can always return an array Edited January 29, 2013 by thescientist

[jimfog]

pass it as an argument into the function. The reason globals are evil is just for that reason, they are global, and that means any script, function, method, etc can get its hands all over it, making debugging and testing a pain. if there are "contracts" for data in, and data out, you can limit the scope of your efforts to just one function, or piece of code.

I understand...at least in theory...you are convincing here.

I would go so far as to create a validation class (static, most likely) that has methods for all the common types of validation you may have in a form, like name, address, email, etc. Then to add validation support to any script, you just include that class and then simply call methods on it and pass your string to it, and get true/false back. Simple, clean, portable, maintainable, and even more so, testable.

Very good suggestion. I do not know OOP well though, and making the code more OOP at this point in time will delay(I think) the finish of the app...which must be done hopefully till summer. It is mine app, not of a client's.

you can always return an array

Good suggestion....but things become complicated...maybe.

[jimfog]

I am at a point in the code where a function returns a value and I wanted to test what you are saying above-that avoid using the global keyword and returnthe variable I wish. Here is it : function get_personal_db ($username)

{  $persdetails=array();..................................................................................          return $persdetails;             }  if($persdetails)             {echo $persdetails['name'];}

Nonetheless...I get the usual variable undefined message.Using the GLOBAL keyword the code works.I though that you do not need GLOBAL when you return the variable. What am i missing here?

[thescientist]

I can't see any of the relevant code. just a bunch of dots and poor indentation. Nor are you being specific about the variable referenced by the error message, or the one you say makes it work when you make add the global keyword to it. Once some basic context and information is provided, I'm sure we can help you out better.

Edited February 4, 2013 by thescientist

[Ingolme]

The return value of a function is given to the function caller.

$x = get_personal_db($username);if($x) {    echo $x['name']}

[jimfog]

The return value of a function is given to the function caller.

$x = get_personal_db($username); if($x) { echo $x['name'] }

yes...now it makes sense. I have already adjusted the code according to the above and it works ok.