Checking for 0

[doubledee]

Sorry for being dense today...

 

How can I rewrite this code...

//HERE
$trimmed['birthYear']=0;
	// Validate Birth Year.
	if ((!empty($trimmed['birthYear'])) && ($trimmed['birthYear']!=0)){
		// Birth Year Exists.
		if (($trimmed['birthYear'] >= $oldestYear) && ($trimmed['birthYear'] <= $newestYear)){
			// Valid Birth Year.
			$birthYear = $trimmed['birthYear'];
		}else{
			// Invalid Birth Year.
			$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
		}
	}//End of VALIDATE FIRST NAME

 

 

...so that if the Birth Year is ZERO - which I am forcing during development - that I get to the ELSE branch of...

 

			$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;

 

 

Apparently "0" is being treated as "Empty" and so my code isn't working?!

 

Thanks,

 

 

Debbie

 

 

[doubledee]

To clarify, when I have a test value of...

 

$trimmed['birthYear']=0;

 

My "User Details" Form keeps getting submitted submitted successfully, when it should fail, becuase a Birth year = 0 would not fall between the years of 1912 and 1983 which is another way of saying "Users must be between 18 and 100 years old."

 

 

I also tried this, but the code still succeeds when it should fail...

 

		if ((!empty($trimmed['birthYear'])) || ($trimmed['birthYear']=0)){

 

 

Thanks,

 

 

Debbie

 

[mrMarcus]

Setting 'birthYear' to 0 will not pass your first condition as you stated.

 

Why must 'birthYear' be 0 for testing?  I wouldn't suggest writing different conditions for development that will need to be changed for production.  Instead of rewriting your code to accommodate development, try rethinking your test methods.

 

Can 'birthYear' be set to 1 for your testing needs?  That will (obviously) fall outside of any birth year you will be allowing on your site.

[Pikachu2000]

Why not just check that it's between $oldestYear and $newestYear to begin with?

[doubledee]

Setting 'birthYear' to 0 will not pass your first condition as you stated.

 

Why must 'birthYear' be 0 for testing?  I wouldn't suggest writing different conditions for development that will need to be changed for production.  Instead of rewriting your code to accommodate development, try rethinking your test methods.

 

Can 'birthYear' be set to 1 for your testing needs?  That will (obviously) fall outside of any birth year you will be allowing on your site.

 

You're missing the point...

 

I am trying to test for "edge conditions" that could break my code.

 

I want to make sure if someone hacked the Form or whatever, and a "0" is sent to my Form, it says, "Stop!  Your Birth Year must be between 1912 and 1983."

 

And, yes, a Birth Year = 1 should fail the same way.

 

Thanks,

 

 

Debbie

 

 

[scootstah]

Use the "identical" operator (===) instead of the "equal" operator (==) to make sure they are the same type. PHP evaluates 0 (int) to false (bool). If you use "===" PHP will compare the int to an int and not a boolean.

[mrMarcus]

My "User Details" Form keeps getting submitted submitted successfully

 

And you should never allow your query to execute if your mandatory conditions are not met.  Something simple like this would suffice:

 

 

Pseudo code

$error = false;

// start checks
if (empty($trimmed['birthYear'])) {
$error = true;
}
if (empty($trimmed['birthDay'])) {
$error = true;
}
// .. and so on
// then...

if (!$error) {
// no you can run your query because there are no errors
}

[mrMarcus]

Setting 'birthYear' to 0 will not pass your first condition as you stated.

 

Why must 'birthYear' be 0 for testing?  I wouldn't suggest writing different conditions for development that will need to be changed for production.  Instead of rewriting your code to accommodate development, try rethinking your test methods.

 

Can 'birthYear' be set to 1 for your testing needs?  That will (obviously) fall outside of any birth year you will be allowing on your site.

 

You're missing the point...

 

I am trying to test for "edge conditions" that could break my code.

 

I want to make sure if someone hacked the Form or whatever, and a "0" is sent to my Form, it says, "Stop!  Your Birth Year must be between 1912 and 1983."

 

And, yes, a Birth Year = 1 should fail the same way.

 

Thanks,

 

 

Debbie

 

 

 

Simple.  Don't use nested condition blocks.  Test each condition separately and handle errors accordingly.

[doubledee]

Why not just check that it's between $oldestYear and $newestYear to begin with?

 

That's something obvious I didn't think of?!  *LOL*

 

But what if birthYear = NULL??  (From my testing, your idea would still work, but what do you say?)

 

Also, is there anything else that would break my code if I just have...

 

		if (($trimmed['birthYear'] >= $oldestYear) && ($trimmed['birthYear'] <= $newestYear)){
			// Valid Birth Year.
			$birthYear = $trimmed['birthYear'];
		}else{
			// Invalid Birth Year.
			$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
		}

 

 

Thanks,

 

 

Debbie

 

 

[scootstah]

But what if birthYear = NULL??

 

Test it.

[doubledee]

Simple.  Don't use nested condition blocks.  Test each condition separately and handle errors accordingly.

 

 

Interesting suggestion.

 

I guess I just set off checking my User Details Form using this kind of coding style...

 

	// Validate Gender.
	if (!empty($trimmed['gender'])){
		// Gender Exists.
		if (($trimmed['gender'] == "1") || ($trimmed['gender'] == "2")){
			// Valid Gender.
			$gender = $trimmed['gender'];
		}else{
			// Invalid Gender.
			$errors['gender'] = 'Gender must be "Male" or "Female".';
		}
	}//End of VALIDATE GENDER

 

 

Debbie

 

 

[doubledee]

But what if birthYear = NULL??

 

Test it.

 

If you read what I said, I said I did test it, but I was asking for some else to confirm my logic was correct... 

 

 

Debbie

 

 

[scootstah]

If it works then wouldn't it stand to reason that the logic is correct?

[mrMarcus]

Simple.  Don't use nested condition blocks.  Test each condition separately and handle errors accordingly.

 

 

Interesting suggestion.

 

I guess I just set off checking my User Details Form using this kind of coding style...

 

	// Validate Gender.
	if (!empty($trimmed['gender'])){
		// Gender Exists.
		if (($trimmed['gender'] == "1") || ($trimmed['gender'] == "2")){
			// Valid Gender.
			$gender = $trimmed['gender'];
		}else{
			// Invalid Gender.
			$errors['gender'] = 'Gender must be "Male" or "Female".';
		}
	}//End of VALIDATE GENDER

 

 

Debbie

 

 

 

I find nesting conditions becomes a nightmare.  A simple setup like below will serve the exact same purpose as nested, but will allow you to easily handle your conditions and errors, should there be any.

 

$errors = array();

// start checks
if (empty($trimmed['birthYear'])) {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}
if ($trimmed['birthYear'] >= $oldestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
if ($trimmed['birthYear'] <= $newestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
// .. and so on
// then...

if (empty($errors)) {
// no you can run your query because there are no errors
}

 

EDIT: this way, there is no *real* need for else statements as you're simply catching the errors when they occur.  Then, as long as $errors array is empty, you can run your queries/whatever.

[Pikachu2000]

But what if birthYear = NULL??  (From my testing, your idea would still work, but what do you say?)

 

All form data is, by default, a string value. It can not be NULL.

[doubledee]

If it works then wouldn't it stand to reason that the logic is correct?

 

These days I do NOT trust anything I write.

 

I am in such awe and fear of Web Development and the Internet, and I have been so humbled here in how little I know sometimes, that I think I'd need to verify if I turned on the lights if they really were on!!!     

 

Sorry, but these final days of me testing my code and re-visiting old code and testing it, and looking for *security* issues, is really making realize how fricking complicated this stuff is, and has me freaked out to no end...

 

Thanks,

 

 

Debbie

 

 

[scootstah]

$errors = array();

// start checks
if (empty($trimmed['birthYear'])) {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}
if ($trimmed['birthYear'] >= $oldestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
if ($trimmed['birthYear'] <= $newestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
// .. and so on
// then...

if (empty($errors)) {
// no you can run your query because there are no errors
}

 

Sorry, but this is not good advice. Neither of the second conditions should even be run if birthYear is empty. And the second two conditions can be run in one condition, there is no reason for two separate if statements here.

 

In my opinion, this is better:

if (!empty($trimmed['birthYear'])) {
if ($trimmed['birthYear'] >= $oldestYear && $trimmed['birthYear'] <= $newestYear) {
	// ...
} else {
	$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
} else {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}

 

Since you are using "birthYear" as the $errors index you can only have one error associated with that condition anyway. So you might as well save the overhead of unnecessarily running conditionals.

 

 

If it works then wouldn't it stand to reason that the logic is correct?

 

These days I do NOT trust anything I write.

 

I am in such awe and fear of Web Development and the Internet, and I have been so humbled here in how little I know sometimes, that I think I'd need to verify if I turned on the lights if they really were on!!!     

 

Sorry, but these final days of me testing my code and re-visiting old code and testing it, and looking for *security* issues, is really making realize how fricking complicated this stuff is, and has me freaked out to no end...

 

And in most cases you are over-exaggerating the risk. You should be focusing on completing the task AND THEN profile it and unit test it and penetration test it and so forth.

[mrMarcus]

It's really only as complicated as your logic dictates.

 

While logical thinking is generally something people are born with, it can, however, be improved by reading up and TESTING TESTING TESTING.  Test things for yourself.  Don't be scared to ask for help, but always try it first.  Try it 100 different ways.  And take note of how people around here format their code and implement their logic.  You'll start getting those "Ahhhhhh" moments and your coding style will greatly improve, along with your abilities.

[mrMarcus]

$errors = array();

// start checks
if (empty($trimmed['birthYear'])) {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}
if ($trimmed['birthYear'] >= $oldestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
if ($trimmed['birthYear'] <= $newestYear) {
$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
// .. and so on
// then...

if (empty($errors)) {
// no you can run your query because there are no errors
}

 

Sorry, but this is not good advice. Neither of the second conditions should even be run if birthYear is empty. And the second two conditions can be run in one condition, there is no reason for two separate if statements here.

 

In my opinion, this is better:

if (!empty($trimmed['birthYear'])) {
if ($trimmed['birthYear'] >= $oldestYear && $trimmed['birthYear'] <= $newestYear) {
	// ...
} else {
	$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
} else {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}

 

Since you are using "birthYear" as the $errors index you can only have one error associated with that condition anyway. So you might as well save the overhead of unnecessarily running conditionals.

 

 

If it works then wouldn't it stand to reason that the logic is correct?

 

These days I do NOT trust anything I write.

 

I am in such awe and fear of Web Development and the Internet, and I have been so humbled here in how little I know sometimes, that I think I'd need to verify if I turned on the lights if they really were on!!!     

 

Sorry, but these final days of me testing my code and re-visiting old code and testing it, and looking for *security* issues, is really making realize how fricking complicated this stuff is, and has me freaked out to no end...

 

And in most cases you are over-exaggerating the risk. You should be focusing on completing the task AND THEN profile it and unit test it and penetration test it and so forth.

 

You're right.  I rushed the snot out of that example without thinking about it which I do quite often.

[doubledee]

Sorry, but this is not good advice. Neither of the second conditions should even be run if birthYear is empty. And the second two conditions can be run in one condition, there is no reason for two separate if statements here.

 

In my opinion, this is better:

if (!empty($trimmed['birthYear'])) {
if ($trimmed['birthYear'] >= $oldestYear && $trimmed['birthYear'] <= $newestYear) {
	// ...
} else {
	$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}
} else {
$errors['birthYear'] = 'Please enter a birth year between x and y';
}

 

Since you are using "birthYear" as the $errors index you can only have one error associated with that condition anyway. So you might as well save the overhead of unnecessarily running conditionals.

 

Scootstah, your solution is what I was thinking of doing to fix things before bed last night.

 

In looking at your code, though, I just remembered one important thing I left out...

 

"Birth Year" is an optional field.  (You don't have to tell me your age, but if you do, it needs to follow certain guidelines!)

 

 

So my testing for "NULL" was wrong, but testing for "0" still makes sense.

 

And unfortunately this messes up your solution, and is probably why I was going down the path I was last night and the code I posted here that I couldn't get to work?! 

 

 

Debbie

 

 

[doubledee]

In English what I need is this...

 

"You don't have to tell me your Birth Year, but if you type anything in the field, whether -999, 0, 1, 100, 1970, 2112, then the Birth Year needs to fall between the years of 1912 and 1983.  And if you type in "BOGUS" that obviously is wrong as well."

 

 

The problem is that I see "0" as "Not Empty", and so my code should have worked, but PHP doesn't see it that way?!

 

Hope that makes things simpler.

 

Thanks,

 

 

Debbie

 

[doubledee]

I think this is all I need...

 

if (($trimmed['birthYear'] >= $oldestYear) && ($trimmed['birthYear'] <= $newestYear)){
	// Valid Birth Year.
	$birthYear = $trimmed['birthYear'];
}else{
	// Invalid Birth Year.
	$errors['birthYear'] = 'Birth Year must be between ' . $oldestYear . ' and ' . $newestYear;
}

 

 

That seems to handle...

 

//$trimmed['birthYear']=-999;
//$trimmed['birthYear']=0;
//$trimmed['birthYear']='BOGUS';

 

 

Sorry for all of the fuss!! 

 

Thanks everyone!!

 

 

Debbie

 

 

[Psycho]

"Birth Year" is an optional field.  (You don't have to tell me your age, but if you do, it needs to follow certain guidelines!)

 

 

So my testing for "NULL" was wrong, but testing for "0" still makes sense.

 

Why would you pass a value (e.g. 0) for a field that doesn't require a value? It would be more logical to set the value for no selection to an empty string. But, if the value is not required you may need to add some special handling for the DB insertion code. The one thing no one has mentioned, however, are decimal value such as 1995.3. A greater than/less than check would pass that value. For this you could add a check using ctype_digit(). Here's my take:

if(empty($trimmed['birthYear']))
{
    //The value is empty, set to logical NULL for DB insertion
    $birthYear = NULL;
}
else
{
    if(!ctype_digit($trimmed['birthYear']) || $trimmed['birthYear']<$oldestYear || $trimmed['birthYear']>$newestYear )
    {
        //The value is not a whole number or is outside the accepted range
        $errors['birthYear'] = "Birth Year must be between {$oldestYear} and {$newestYear}";
    }
    else
    {
        //The value is a valid year in the accepted range
        $birthYear = $trimmed['birthYear'];
    }
}

[doubledee]

Why would you pass a value (e.g. 0) for a field that doesn't require a value?

 

As mentioned before, because I am just making sure my code handle outlying conditions that a hacker might type in.

 

 

It would be more logical to set the value for no selection to an empty string. But, if the value is not required you may need to add some special handling for the DB insertion code.

 

 

I should have included this, but here is the code for my drop-down for "Birth Year"...

 

<!-- Birth Year -->
<label for="birthYear">Year Born:</label>
<select id="birthYear" name="birthYear">
	<option value="">--</option>
	<?php
		// Display dates for Users between 18 and 100.
		for($year = $newestYear; $year >= $oldestYear; $year--){
				$selected = (isset($birthYear) && $birthYear == $year) ? ' selected="selected"' : '';
				echo "<option value='{$year}'{$selected}>{$year}</option>\n\t";
		}
	?>
</select>
<?php
	if (!empty($errors['birthYear'])){
		echo '<span class="error">' . $errors['birthYear'] . '</span>';
	}
?>

 

 

 

The one thing no one has mentioned, however, are decimal value such as 1995.3. A greater than/less than check would pass that value. For this you could add a check using ctype_digit(). Here's my take:

if(empty($trimmed['birthYear']))
{
    //The value is empty, set to logical NULL for DB insertion
    $birthYear = NULL;
}
else
{
    if(!ctype_digit($trimmed['birthYear']) || $trimmed['birthYear']<$oldestYear || $trimmed['birthYear']>$newestYear )
    {
        //The value is not a whole number or is outside the accepted range
        $errors['birthYear'] = "Birth Year must be between {$oldestYear} and {$newestYear}";
    }
    else
    {
        //The value is a valid year in the accepted range
        $birthYear = $trimmed['birthYear'];
    }
}

 

Psycho, I think you get the golden prize for your code!! 

 

I think you are handling outlying cases better than anything discussed so far, and I like you handling of decimal values as well.

 

Thanks!!!

 

 

Debbie

 

 

[doubledee]

P.S.

 

Can you explain what you are doing with the curly braces {} inside this line of code...

 

       $errors['birthYear'] = "Birth Year must be between {$oldestYear} and {$newestYear}";

 

Thanks,

 

 

Debbie