dispatch_address_postcode
isn't mandatory and it will still run even if it's blank:
if (!is_null($_POST['personal_info_first_name']) &&
!is_null($_POST['personal_info_surname']) &&
!is_null($_POST['personal_info_email']) &&
!is_null($_POST['personal_info_telephone']) &&
!is_null($_POST['dispatch_address_country']) &&
!is_null($_POST['dispatch_address_first_name']) &&
!is_null($_POST['dispatch_address_surname']) &&
!is_null($_POST['dispatch_address_address']) &&
!is_null($_POST['dispatch_address_town']) &&
!is_null($_POST['dispatch_address_postcode']) &&
!is_null($_POST['dispatch_address_county']) &&
( ($_POST['payment_method'] == "Pay by credit card.") ||
(
($_POST['payment_method'] == "Pay by new credit card.") &&
!is_null($_POST['card_number']) &&
!is_null($_POST['expiration_date']) &&
!is_null($_POST['security_code'])
)
)
)
What gives?
"dispatch_address_postcode isn't mandatory and it will still run even if it's blank…"
Just look at that sentence again. If the field is not mandatory, it is perfectly okay if the code runs if the field is blank. If a field isn't mandatory, don't test it as mandatory.
The real problem is though, is_null only tests if the variable is null. POSTed values will never be null, if they're empty they will be '' (an empty string). All your !is_null tests will always be true, and you will get a warning if the variable isn't set (something you don't want to happen). The more appropriate test would be !empty.
Even more appropriate tests would include a test if the value appears to be valid (does email look like an email address, does telephone have at least x digits in it?). You should also loop through the fields to make your code more readable, endless nested and chained if conditions are no joy to look at.
$mandatoryFields = array('foo' => 'email', 'bar' => 'telephone');
foreach ($mandatoryFields as $field => $rule) {
if (empty($_POST[$field]) || !validateByRule($_POST[$field], $rule)) {
raiseHell();
}
}
It looks like you're trying to make sure all post variables are submitted. Would you like help with that?
Using !empty() may not be the answer to your specific question, but it would definitely help with what it looks like you're trying to do.
empty() returns TRUE if the $_POST key isn't set, if its an empty array, or even if its an empty string, so using !empty() is a good way to make sure that the user has filled in the information.
Try writing your own is_valid function and use that rather than is_null.
For example (and this is by no means comprehensive):
function is_valid(&$array, $key, $required=false) {
if(!array_key_exists($array))
return false;
$value = trim($array[$key]);
if(empty($value) && $required)
return false;
return true;
}
Use like so:
if(is_valid($_POST, 'personal_info_first_name', true) && ...)
!is_null($_POST['personal_info_first_name']) && !isset($_POST['personal_info_first_name'])
use array_key_exists('card_number', $_POST) && !empty($_POST['card_number'])
Edit: Please consider this before a downvote. I'm leaving this here to serve as a "what not to do". I would delete it because it's bad, but then nobody would learn from my mistakes.
DO NOT DO THIS - read the comments for great info on why this is bad
My answer is going to be wildly different, but I am a wildly different guy...
I JUST found that this will work. Instead of all that isset and things, just assign the variables programmatically! I think I have some refactoring to do... y'know on all my code...
if (!is_array($_POST)){exit "$_POST isn't an array";}
foreach ($_POST as $param => $value){
${$param} = secure($value);
}
//now you have a set of variables that are named exactly as the posted param
//for example, $_POST['personal_info_first_name'] == $personal_info_first_name
if ($payment_method == "Pay by credit card."){
//do stuff that you were gonna do anyways
} else if ($payment_method == "Pay by new credit card.") {
if ($card_number && $expiration_date && $security_code){
//do stuff that you were gonna do anyways
} else {
exit("info missing for credit card transaction");
}
} else {
exit("unknown payment method")
}
function secure($input){
//sanitize user input
}
If you use this code, then it doesn't matter what is null and what isn't within the foreach because anything that's null just won't be made. Then you can use nicer looking code (and probably faster code) to check for anything that is required.
