When a Function Fails

As a rule of thumb, what should a custom function return when it fails?

Is the standard that it should return FALSE?

Or is it okay - based on context - if it simply returns nothing?

I just wrote a function which takes a decimal and splits it up into the whole and fractional parts, but what should it return if you pass it a NULL or “Hello”? :-/

As it stands, I just returned an array with two NULL subcomponents - the thinking being that whole and fractional parts of “Hello” are nothing!

Sincerely,

Debbie

There really isn’t any standard usage of what is expected from a function, by default or otherwise…sometimes you will use a function that returns nothing at all, you use it just to define something else ( come to think of it, that’s more or less what interfaces are in PHP ). If you are coding procedurally, one of the main uses of functions are to simply avoid code repetition, which is what you are doing above. In other words, you are overthinking things here! Which is a good thing…

I think throwing an exception is the best option.

I’m not a fan of returning false or null because

  1. Real return values vs error return values get mixed together.
  2. You’re forced to always double check the return value:
$parts = decimal_split(123.456);

# Before I can safely use $parts, I need to check that it isn't false or null or whatever
# And I need to do this every time I call that function, otherwise I might never catch a potential error
if ($parts === false) {
    # What now? Should I re-communicate the error up another level?
    return false;
}
  1. And what does “false” even mean? Was one of my arguments the wrong type? Was there some other argument that I forgot to pass in? Or was there a problem with the kind of calculation I asked it to do? “false” tells me something went wrong, but it doesn’t tell me specifically what went wrong.

It seems to me that exceptions solve all these problems. 1) Your return values and your errors aren’t mixed together, at all. 2) You can choose not to catch an error and it will automatically bubble up the call stack to a level capable of handling that kind of error. 3) You can say specifically what went wrong, through a combination of the exception type and the message.

First off, thanks for the response, Jeff. (As always seem to challenge me.)

You make some good points, BUT you also just opened up a whole can of worms that I am unable to handle at this time! :frowning:

I just wrote this function because I thought it might be a way to keep my 2,300 lines-of-code script from hitting 2,400 lines-of-code!!

In this instance, it probably makes sense for me to just make this function do what it needs to for this particular situation and call it quits.

What you are talking about sounds exciting, BUT it also sounds like a “white elephant” I have been putting off, which is my need to learn how to be a more efficient coder… :frowning: :frowning: :frowning:

It seems like your advice might help more if I was doing object-oriented programming, right?

Sincerely,

Debbie

Not just for OOP, what he’s suggesting is useful anywhere. Although in your particular example, I think it’s a little bit overkill…exceptions can be avoided altogether (which is ideally your goal any time, if at all possible) in this instance with proper type casting and a small rewrite of the script that we were talking about in the other thread…you should have kept this question in that thread, that way it’s easier to keep track of what’s going on, and for the people who haven’t seen the other thread

@Jeff_Mott ; you have brought up a really deep and interesting topic!

Let’s take a deeper look at my innocent getSplitDecimal() function…

I originally wrote this function just as a way to hide some code in my already bursting 2,300 lines-of-code “article.php” script. Nothing more.

But then after some time, I decided this function could be useful in other places, so I started putting some more effort into it.

And now I feel conflicted about how to best write this function. So, stepping back…

In and of itself, getSplitDecimal()'s job should be to take a DECIMAL number and return the Whole portion and the Fractional portion.

If you pass it a String, then I guess it should throw an error. But if you pass it a NULL or Empty String, I’m not so sure. (The Whole and Fractional portions of a NULL or Empty String are also NULLs, so why not just return nothing?)

For the purposes of building my Star Rating, it will be common that a Comment will have no $avgRating, and so the function would have to deal with NULLs.


My question to you is this…

How restrictive and Draconian should you make a Function?

On one hand, it is great if it throws an error every time there is some issue.

Then again, do you want your Function to barf and bring your whole website to a grinding halt over something petty?! :eek:

I have some Functions (e.g. validateSurveyResponse) that absolutely should throw up an error to the user if there an issue (e.g. they hacked the Form and submitted a bogus response).

And then there are other Functions (e.g. getAgeRange) which takes a Date-Of-Birth and returns a more modest “20-something” that if it breaks I don’t really care and will just have it display a “–” in that location on the page.

Follow me??


Back to my getSplitDecimal()

I think in all cases, if you pass a String to that Function it should complain and throw an error. But I am not so sure that it makes sense to program into the Function an Error for a NULL or Empty String.

In my current application, I would rather do this…


	if (!empty($ratingAvg)){
		// Rating Exists.
		// Check Range.
		if (($ratingAvg >= 1) && ($ratingAvg <= 5)){
			// Valid Rating.
			$getSplitDecimal = getSplitDecimal($ratingAvg);
			
			// Start building Stars here...
			$stars = ...

		}else{
			// Invalid Rating.
			$stars = '';
		}
		
	}else{
		// Rating does Not Exist.
		$stars = '';
	}

As I see it, it is really the $ratingAvg that I should be sanitizing and validating more so than the results from getSplitDecimal().

The $ratingAvg is coming from a query in MySQL, so it is basically impossible that it could be “Hello”. And if $ratingAvg was a NULL (i.e. nobody has rated the Comment), then I think I need to handle that outside of my getSplitDecimal() function, because if $ratingAvg is NULL then I need to set $stars = ‘’; so that no Stars appear, and that really has nothing to do with this function’s purpose.

Follow me?

So… What are your thoughts on all of this??

I can totally see your earlier points, but then again I can also see where if you “go down the rabbit hole” too quickly and make your Functions too restrictive, that it can work against you…

Sincerely,

Debbie

If you don’t have a value to pass into the function, then you shouldn’t be calling the function.

The function expects a decimal argument, so if you pass in a non-decimal argument, then the function should “barf”.

Exactly.

Aren’t you talking out of both sides of your mouth here? :slight_smile:

On one hand you imply that my function should handle all scenarios and throw errors accordingly.

On the other hand you say that my code surrounding the function should be catching things.

So which approach is better overall?

I have long struggled with this concept…

How much “redundancy” is necessary in code?

Do you keep checking and checking for stuff in the beginning all throughout your code, OR do you check things once up front, and them assume that they haven’t changed along the way?

What did you think about my code snippet in Post #6?

Sincerely,

Debbie

Both. :slight_smile:

One is doing the right thing. The other is enforcing the right thing.

If your function takes a decimal and all you’ve got is a null, then you shouldn’t be calling that function.

But, if you do, then your function could catch that mistake and throw an error.

Probably less than what you seem to be aiming for. :wink:

Some languages are strongly typed, so that if you declare a parameter’s type as a number and someone passes a string, then the language itself will throw an error without you having to do anything special.

But PHP is not a strongly typed language. I’ve seen lots of attempts in the past, for both PHP and JavaScript, where people try to simulate strong typing. But I think I’ve bounced between enough languages to learn an important lesson: Don’t fight the language. Use it the way it was designed to be used. If PHP is loosely typed, then… that’s just how it is.

That second choice sounds about right.

You probably don’t need to check for an invalid rating. It’s coming from your own database, after all. But otherwise everything looks fine.

Okay.

Fair enough.

So this is a whole other topic, but to give you the “short-version”…

Debbie’s “error-handling” consists of finding some condition that is wrong, then sticking the script name and an Error-Code into the Session and calling my “outcome” script which displays a custom message related to said error.

Some people have said that isn’t really “error-handling”, but it seems to work for now.

To further complicate things, I created a variant of the above scenario for Functions.

With most Functions, I simply log the error, because I have decided most function failures do not justify ending the parent script.

And this leads me back to my - maybe you could call it an “aversion” - concern about error checking every little thing in my Functions for fear of killing my website.

Follow my mad thinking?

OBVIOUSLY how I have chosen to handle errors is-what-it-is for v2.0, but I also want to improve things now and in the future.

So it sounds like the code around my getSplitDecimal() should catch things like NULL, but to do it really right, my Function should throw an error for NULL, Empty String, and Strings, right?

Saying I am being too much of a perfectionist? :slight_smile:

I know I am probably a little too obsessive on this point!

Okay, good!

Sincerely,

Debbie