What do you usually test for first: true or false?

Bonesnap

Just something I've been wondering about lately and I have been trying to think of a consistent way of approaching it, but wanted to hear what others did in similar situations and if there was any reason behind it.

Quite frequently when coding you have come across a situation where you have to test a boolean. Say you were looping through an array and displaying some tabular data. You would first check to make sure the array wasn't empty, and if it was, then display a message indicating there was no data to display, otherwise loop through the array and display the data. But how do you test it? Do you test for the positive or the negative (true or false)?

Just a quick visualization for what I mean:

//Testing for true (positive)
if(empty($array))
{
    //Echo out some message to the user
}
else
{
    //Loop through array
}

//Testing for false (negative)
if(!empty($array))
{
    //Loop through array
}
else
{
    //Echo out some message to the user
}

Is there any particular approach you take? Does it depend on the situation? Is it better to do one way over the other? It seems like a non-factor but I'm always curious on how others approach these kinds of things.

NogDog

While I'll freely break this rule whenever I feel (rightly or wrongly) that it makes the code clearer, the general rule I start from is to code the normal path first, then handle the exceptions afterward:

function bar($param, &$error='')
{
	$result = false;
	if(is_numeric($param)) {
		if($param > 0) {
			// do something unbelievable with $params
			$result = true;
		}
		else {
			$error = 'param cannot be <= 0';
		}
	}
	else {
		$error = 'param must be numeric';
	}
	return $result;
}

So it's not a case of true vs. false as much as normal vs. abnormal.

bradgrafelman

I switch between first checking for the condition that I suspect is true the majority of the time and the condition where I'm checking if something equals zero. Both stem from optimizations for embedded code where I want to write the most optimized/efficient code (turns out there's a lot you can do other than throwing a "-O2" on the command line for your compiler and hoping for the best 🙂).

dalecosp

bradgrafelman;11034437 wrote:
(turns out there's a lot you can do other than throwing a "-O2" on the command line for your compiler and hoping for the best 🙂).

Yup; learned that, somewhat, doing contests here all those years ago. Only problem being that in Web development management often does something like this:

$wish = "Last Week";
define("SOON","Today");
//etc....

And, for the original question:

I'm not certain. I generally think that I'm trying to avoid errors in my coding, therefore, if I'm coding a loop to play with an array, I will make sure that (!empty($array)) because I want to loop to succeed. Similarly (I think), I've noted that if I expect a value from a variable, I'll usually code the truth section first:

if ($some_variable) {
   do_it();
} else {
   //error code...
}

I believe that comes from lots of code examples when we're learning at first, doesn't it?

Bonesnap

Thanks for the replies, guys. Looking through some old projects and websites I have done it looks like I'm kind of a mixed bag with the way I approach it, with a slight tendency to approach it like NogDog. Moving forward I think I'll add that to my "things I always do" while coding to keep it consistent. It's interesting looking back at old sections of code because I can tell how I was thinking at the time depending on what I was testing for first.

dalecosp

Bonesnap;11034593 wrote:
Thanks for the replies, guys. Looking through some old projects and websites I have done it looks like I'm kind of a mixed bag with the way I approach it, with a slight tendency to approach it like NogDog. Moving forward I think I'll add that to my "things I always do" while coding to keep it consistent. It's interesting looking back at old sections of code because I can tell how I was thinking at the time depending on what I was testing for first.

It is interesting looking back. Someone posted this recently: 7 reasons I switched back to PHP after 2 years on Rails. Interesting quote:

Derek Sivers wrote:
But the main reason that any programmer learning any new language thinks the new language is SO much better than the old one is because he’s a better programmer now!

johanafm

Bonesnap;11034593 wrote:
because I can tell how I was thinking at the time depending on what I was testing for first.

One thing I find from time to time, especially when looking at older code, is testing for whatever results in a shorter code block coming first

if (true|false) {
    # 1-2 lines of code
}
elseif (the-other-condition) (
    # 1-2 pages of code&#8230;
}

While I still rather see the beginning of both code blocks on screen at the same time than having to scroll and scroll and scroll to reach the second, today I'd would definitely refactor the code if I needed to make one single other change to it.

Other than that, I try testing for end condition first, if applicable

if ($done) {
    return;
}
# do more stuff code before returning

sneakyimp

I find myself checking if something is NOT there first and this more often than not results my code exiting the current function or throwing an exception. I think the primary goal is to avoid a situation where your code's logic results in deeper and deeper and deeper sets of brackets. It becomes impossible to read and maintain.

// NO!!
function mybadfunc() {
  if ($var) {
    // do something fancy here
  } else {
    // don't do anything here?
  }
}

// YES
function mygoodfunc() {
  if (!$var) {
    throw new Exception("var is empty.");
    // or return
  }

  // do something fancy
  // note that this code is not indented!

}

NogDog

Of course, there are those who insist a function/method should only have one return, who would then shudder at the thought of testing for an error and immediately returning if found. (I've been known to do it, but then nobody has ever accused me of being any type of coding purist.) Also, depending on what you are checking for, said purists would say if it's something like validating user input, it's not an exception, so you would not throw one for that situation. (I'm probably more closely aligned with that camp than I am the single-return camp.)

traq

<-- not in the single-return camp.

Conditional returns are better than nested statements, IMO. I'll usually do whatever tests will throw me out of the function quickest at the beginning. Usually that will actually result in an exception, but there are cases where valid input will result in no action, so I just return early. I do try to have only one "successful" return statement, and I try to keep it at the end. This is mostly for readability, I suppose.

Testing for true/false first is similar: generally speaking, I test first for whichever condition will avoid the most code execution.

NogDog

As they like to say in the Perl world: TMTOWTDI. 🙂

johanafm

NogDog;11034637 wrote:
Of course, there are those who insist a function/method should only have one return, who would then shudder at the thought of testing for an error and immediately returning if found.

Would you care to elaborate / point to (re)sources? I have trouble coming up with any but three possible scenarios
1. test for error, return without indication => agreed, sounds bad
2. test for error, return with indication of this
3. test for error, throw exception

NogDog;11034637 wrote:
Also, depending on what you are checking for, said purists would say if it's something like validating user input, it's not an exception

And why not? Please elaborate, or point me to other (re)sources.

As I understand exceptions and their usage, it goes a little something like this:
1. when something is wrong, throw
2. if you can handle -> do so!
3. if not, log and rethrow (so the the exceptions are tiered just like the rest of the architecture and matches the same abstractions)

Let's say your code ends up doing this

$db = new PDO;
$db->query("INSERT INTO table(varchar2) VALUES('abc')");

The initial error would be a 22001 - data right truncated. Probably translated into "data too long for column 'varchar2'", which is turned into a PDO exception. Whatever the data layer receieves, it should definitely not expose pdo exceptions to the model. The best thing it could do would be to thow SomeDataLayerException which, in turn, would provide the model with enough information to, in turn, provide the view with

[b]something[/b]* the view can use to somehow give the user appropriate feedback, such as indicating the offending input field

* So what should "something" be?

What makes more sense than throwing an exception? The error could be detected in two ways. The first is "while validating input, before trying to store" and the second is "oops, something went wrong further down the chain and they let me know - by throwing". If detected by catching an underlying exception, the proper course of action would be to log the error and rethrow (its own kind of exception, not a data layer exception), so that the model might handle it. If detected during validation, throw without logging. Having two ways of letting the view know "username is too long" makes no sense. The user doesn't care who detected the problem, and thus, the view has no reason to.

Moreover, the reason you throw exceptions is so that if no point along the call chain knows how to handle the exception, the program should terminate with a big flashing "WTF!?". Not being able to create an account due to a too long username certainly seems like something a user should be informed about. If not nicely, then reasonably through an "Abnormal program termination. Unhandled exception 'Validation' with message: username too long".

And just to be clear, I'm not claiming that you should "always throw exceptions". For example, let's say someone tries to create a new user, with a too long username, by sending a POST request to your REST api. Wether you detect that during validation or you detect it due to the 22001 data right truncated error being propagated through various exceptions up the chain, I'd still expect both to end in a 400 Bad Request.

Where's the difference between that and an exception thrown by the model and caught by the view when they both reside within the same architecture?

NogDog

I'm just going off of things I've read -- not necessarily from the finest sources -- over quite a few years. Like I say, I don't necessarily follow them, just use them as references when I catch myself breaking a "rule" to at least think twice about it before I go ahead and do it anyway.

My local-scope understanding of exceptions has always been that they are supposed to be for handling situations that shouldn't happen, but just in case they do happen, you're ready for it. To my mind, a user neglecting to enter data in a required field is not a program exception, just a reasonably expected user error that I want to check for and deal with. I personally would rather return false from my input validation function and let the controller code (or whatever) that called that function decide what to do when it gets such a result. (And when the controller calls that function and gets a false result, that's a reasonably expected result, not something "exceptional".)

That being said, there is a certain usefulness when working in a multi-tiered design of using the ability of exceptions to filter up through the stack until something catches it, so I can see a case for leveraging that ability in some situations that perhaps I would not define as "exceptional".

As far as multiple returns, if you search on "single exit point", you'll find lots of stuff -- much of it indicating that few people care much about it these days. 🙂

But like I tried to infer, I don't think there is one set of correct rules for all of this. I think the important thing is that we are thinking about it, so when we write code that throws an exception or returns early in a function, we at least pause for a few milliseconds to consider if that's really the way we want to do it, or if there's a better way to write it that will make more sense to us a year from now when we have to debug that code.

Weedpacket

johamafm wrote:
Would you care to elaborate / point to (re)sources? I have trouble coming up with any but three possible scenarios
1. test for error, return without indication => agreed, sounds bad
2. test for error, return with indication of this
3. test for error, throw exception

Nicholas Wirth enforced this (thou shalt not have more than one point of return in thy function or procedure) in his design of Pascal.

NogDog

It can seem a bit forced if you try to stick with a single exit point. You tend to end up with something like:

function myFunc($param, &$error)
{
	$result = false;
	if(is_int($param)) {
		if($param != 0) {
			if($param %2 == 0) {
				$result = doSomethingAmazingWith($param);
			}
			else {
				$error = "Must be even";
			}
		}
		else {
			$error = "Cannot be zero";
		}
	}
	else {
		$error = "Must be an integer";
	}
	return $result;
}

Then again, when you look at that, you can see the "normal" flow of what happens right away -- it's figuring out what happens when any of those checks fail that gets a bit messy. Then again, especially if it gets even more levels of nesting, maybe that's a sign that some of that validation stuff could be moved into a separate function? Then it might just be:

function myFunc($param, &$error)
{
	if(($result = validator($param, $error)) == true) {
		$result = doSomethingAmazingWith($param);
	}
	return $result;
}

function validator($param, &$error)
{
	$result = true;
	if(!is_int($param)) {
		$result = false;
		$error = "must be integer";
	}
	elseif($param == 0) {
		$result = false;
		$error = "Cannot be zero";
	}
	elseif($param %2 != 0) {
		$result = false;
		$error = "Must be even";
	}
	return $result;
}

Do I always code that way? Nope. But I do think about it and do follow that pattern when it seems to work smoothly. Other times it seems clumsy or overkill, and I have multiple returns, etc. 🙂

traq

I've been waffling back and forth over whether or not to build a parameter validation method(s)...

I guess it's a good idea, but I would have to rewrite soooooooo much at this point...

Weedpacket

What I sometimes do is to separate parameter validation out from the workings of the method so that I can call the method without having to do parameter validation (I don't try and write a generic "parameter validation" method because what qualifies as a valid parameter varies from method to method).

public function doSomething($params)
{
    // validation stuff
    $this->_doSomething($params);
}

private function _doSomething($params)
{
    // really do stuff.
}

The other methods in the class can call the private method on the assumption that "yes, I know what I'm doing, the parameters are fine - I've already checked...."

traq

That's a good idea.

I've been trying to teach myself to find the "last minute" point of validation for any given parameter (e.g., I have a method that uses/proxies/whatever another; so it should ignore validation and just let that other method worry about it).

Bonesnap

I have encountered a few people who subscribe to the "only one return statement" philosophy, to the point that they tell me "that's the proper way of doing it".

I personally don't follow that, but if the function or method ends up that way, then so be it, but I'd rather not have a ton of nested if statements like NogDog's example.

On the topic of exceptions... I haven't used them that extensively. Is that bad? I usually return false or some string (depending on the situation - but usually false) when there's unexpected behaviour.

NogDog

Bonesnap;11034767 wrote:
...
On the topic of exceptions... I haven't used them that extensively. Is that bad? I usually return false or some string (depending on the situation - but usually false) when there's unexpected behaviour.

For me, exceptions are for the sort of "unexpected behavior" that would result in a "WTF?" response, as opposed to, say, something that shouldn't normally happen but I can easily envision happening in some situations (e.g. bad user input). If my PDO::__construct() fails, that's probably a good candidate for an exception, since in theory it should always work unless the DB server is down, someone has changed the password -- things that are definitely not "normal". But as with much of the other neat stuff we've been discussing, I personally take any such rule as a rule of thumb, freely deviating from it when I feel it makes for a cleaner, easier to maintain code base. For instance, it might make sense in some application to throw a UserInputException if validation fails, if the framework being used would allow you to catch it somewhere up the stack where you could use it to redisplay the form with the data in that exception object being used to display the error to the user. (Not sure I'd ever do that, but I can see a potential use case for it.)