Always Return Something

This post has been updated since it was first published.

A few weeks ago, there was a discussion on Twitter about whether or not a method should always return a value, or whether or not null was a valid value to return. The answer to this question is a resounding no, a null value should never be returned.

You should always return something when writing a function or a method. Here’s why.

A return value can be used to tell if the function behaved properly.

When you’re writing code, you should always do a sanity check to verify that the functions you’ve executed have been successful. For example, you check that a file you opened exists, or that a resource performed correctly before using it. But if you receive a null response, how do you test for this?

The answer is you can’t. You can’t check whether or not the response was valid because the response doesn’t give any indication. Even if a failure results in a null response (which many native PHP functions do), you can’t know for sure if you received a null response due to a failure of the function or some API change that broke the function between releases.

Having an expected return value eliminates this possibility and makes sanity checking in your code possible.

A null return value can’t easily be tested.

Just as your code can’t execute a proper sanity check, a null value prevents proper unit testing. Imagine that you’re testing a setter and you receive a null response; how then, do you know that the setter worked properly?

You can invoke the getter to check the object, but that creates an unnecessary dependency and doesn’t truly test the setter. With the null response you have no way of knowing that the function executed properly, and that some other developer didn’t insert a random return statement into the code somewhere.

With a return value, even a true or false value (for success or failure), you can have a greater level of confidence that the function is performing as expected. Changes in that return value will break the test, resulting in a failure, without having to invoke another function and compromise the atomic nature of the test.

Returning a null value breaks fluent interfaces.

Many developers like to use getters and setters in a fluent way. For example:

<?php

$obj->set($key, $val)->set($key2, $val2)->set($key3, $val3);

?>

While fluent interfaces can be a code smell in some cases, using them in setters in this way is perfectly valid and made possible by the return of the $this variable by the setter.

If a developer returns a null value instead, fluent interfaces become impossible. This may be by design, but it can also be a source of frustration for developers who want to chain objects together (for example, in an ORM).

Note that I don’t actually recommend using fluent interfaces. Fluent interfaces create a mess out of your code in most instances. They produce unreadable code that is difficult to test. Fluent interfaces make assumptions about the return values you’re expecting from the method you’re executing. Fluent interfaces are a form of code smell that I generally avoid.

So, what should you return?

If you’re not returning null, what should you return?

Raise an exception

If you ask a method for some data that it doesn’t have, or cannot get for you, raise an exception. An exception is a perfectly valid response when you are asking a method for something that’s either impossible, or reasonable but not available. The exception should be handled by the requestor of the information that isn’t available.

Django does this for many methods. If you request something from the database that’s impossible an exception is raised. It doesn’t “fail silently” with a None response; instead it tells you that what you asked for couldn’t be provided, and expects you to deal with it.

Return an empty set

If you are requesting a data set, you should always return some sort of empty data set rather than null. For example, an empty array is a valid response of there is no data available. If you are returning a data set object, return an empty one (or a special one that implements the correct interfaces). Not only does this make your code more explicit, but it doesn’t break functions or routines that expect a particular type (e.g. foreach() expects an array or other iterable, and will emit a warning on NULL, which you then have to test for).

Return NULL anyway

If you’re pulling a column from a data set that was retrieved from a database, null may well be the only value that’s appropriate. In these cases, knowing that null is an appropriate value makes your code more likely to perform the correct evaluations to determine if that value was appropriate. Every rule has its exceptions, and this is certainly one case where NULL is an acceptable return value.

Brandon Savage is the author of Mastering Object Oriented PHP and Practical Design Patterns in PHP

Jean-Marc Fontaine (@jmfontaine) wrote at 3/12/2013 7:22 am:

The point to unit testing is to make sure a piece of code works as expected.

You must check by yourself. You can not just believe its return value because this value could be wrong.

beberlei wrote at 3/12/2013 7:29 am:

I disagree very much:

1. Functions with return values cannot be made async.

2. You can check for proper behavior with exceptions, mocks and state changes. You make it seem that the return value is the only way to check if behavior worked.

3. The Command-Query-Separation suggests that methods that change state should never return something to make intent clear and APIs simple.

4. APIs designed with your premise get bigger because they return values that are “just required for tests”. Then the next developer comes along and starts using the values and then you have messy code.

Peter wrote at 3/12/2013 8:25 am:

i call it as i see it:
bulls.

in case of error, throw an exception, thats what they are made for.

Jurian Sluiman (@juriansluiman) wrote at 3/12/2013 8:35 am:

I am not really convinced by your arguments. I don’t think it is necessary to return something on each and every function. In your post, it is all about the _expected_ behaviour. Well, I think anything _unexpected_ should not be handled with a return false or something. Anything unexpected should be exceptional and can be handled with an exception.

Normally I follow the Command Query Separation. In these cases, queries return the result and commands return nothing. That makes it clear for anyone what to expect. And if a command fails, an exception is thrown. I would rather not check for every command I call if it did succeed or not: it is up to the callee what “success” means. Is “success” true? Or the object self? Or another object or scalar value? The return value for “success” can be different in each and every case, so you must rely on documentation what to expect. If you know exceptions are thrown in exceptional cases, that makes it clear for everyone.

Jan Schneider (@yunosh) wrote at 3/12/2013 8:57 am:

Uhm, exceptions?

Functions should only return something if:
1) their purpose is to return something
2) building fluent interfaces

For error conditions you use exceptions, not return values.

Jory Geerts wrote at 3/12/2013 9:32 am:

First you say that “A null return value can’t easily be tested.”, but the same is true for a fluent interface – if your getter always returns ‘$this’, the return value doesn’t tell anybody if it worked either.

In my opinion, using exceptions is often a much easier way to indicate a ‘failure’.

Gary Spabby (@GeeH) wrote at 3/12/2013 9:39 am:

I can’t agree that a function should never return null; sometimes null exactly the correct thing to return. Imagine the following function where the database column allows nulls.

function getCommentLinkById($id) {
$row = $db->find($id);
if(!$row) {
return false;
}
return $row[‘link’];
}

mongolito404 (@mongolito404) wrote at 3/12/2013 10:10 am:

NULL is the proper return value when there is no value to return. For instance, when querying the database for a nullable column of for a row that does not exists (and does not need to exist).

The return value of a method (or a function) should not be used for error conditions. If you can’t and don’t want to use exceptions, use a by-reference parameter for either the “returned value” ($error = foo($arg1, $arg2, $value)) or the error ($value = foo($arg1, $arg2, $error)). This allow you to detect errors without checking the “returned value”.

mongolito404 (@mongolito404) wrote at 3/12/2013 10:12 am:

And to be pedantic, NULL is a value in PHP as stated in the documentation: “The special NULL value represents a variable with no value. NULL is the only possible value of type null.” (ref. http://php.net/manual/en/language.types.null.php)

KingCrunch wrote at 3/12/2013 10:21 am:

Oh, please don’t …

If a method doesn’t have to tell something, it _shouldn’t_ tell something.

This fluent-interface just provokes messy code, because it leads to longest method call chains, where nobody can follow, which method returns what and why. Especially “why”. I say “setSomething()” and the method returns itself? Do you always say youre name, when something tells you something.

“Can you give me the sugar?” — “Brandon” — “And a cup?” — “Brandon”

And exactly this is how a fluent-interface looks like ;)

Eduardo Casas (@eduardo_casas) wrote at 3/12/2013 10:41 am:

I totally agree with @yunosh. I think that nothing else is needed to add.

KingCrunch wrote at 3/12/2013 10:57 am:

@eduardo_cases
Well one can add, that fluent-interfaces are crap :)

Rasmus Schultz (@mindplaydk) wrote at 3/12/2013 12:18 pm:

In my experience, these issues most often occur in models, with objects that have optional relations – for example, $user->country->name could fail if the country relation is optional.

In my opinion, solutions to this problem range from bad to worse. The real issue is “old school” domain modeling – like many domain-related issues, this one largely disappears when you stick to modern DDD practices. That, is don’t model $user->country as a relation, because the Country object isn’t part of the User aggregate in the first place – it’s outside the transaction boundary of any reasonable User transaction, so only the Country ID is part of the actual User aggregate, and should be modeled as such.

So now instead of $user->country->name, you get something like $countries->getOrDefault($user->country_id)->name, where $countries is your Country repository. You’re now explicitly saying “give me the Country or a Country null-object”, which is what you want in a scenario where you’re displaying the Country name. In some other scenario (e.g. business logic rather than display purposes) you may want a NULL rather than a null-object, or you may even want an Exception thrown – you indicate this in consumer-code by explicitly calling get(), getOrDefault(), getOrNull(), or whatever is appropriate in each case.

Classic domain modeling (and for that matter most modern ORM) teaches us to ignore transaction boundaries and model everything as monolithic graphs – issues like this one is just one of many issues that go away when you model your domain strictly respecting transaction boundaries. Yes, it takes a little more code – but model-consumer code becomes a lot more transparent, and you can more elegantly handle necessary deviations from the way you generally treat relations, on a case-by-case basis.

DDD practices desperately need to gain footing in the PHP world.

Eduardo Casas (@eduardo_casas) wrote at 3/12/2013 2:18 pm:

@KingCrunch
Well, I don’t agree. :)

1. Fluent interfaces are much more than method chaining and, despite your opinion, they are widely used to improve the readability of the code.

2. Said that, it doesn’t mean we must use them everywhere. But sometimes they’re pretty useful:

“Can you give me the sugar?” — “Anything else?” — “And a cup?” — “Anything else?”

I think this is a better approach, which makes sense.

Mathias Verraes (@mathiasverraes) wrote at 3/12/2013 3:41 pm:

@beberlei and most other commenters hit the nail on the head.

I only want to add (slightly off topic):

– Fluent interfaces for setters are bad practice because they encourage anaemic models. When you see $thing->setFoo($foo)->setBar($bar) , it often means you may need to encapsulate those as one operation doSomething($foo, $bar), where ‘doSomething’ is a verb that has actual meaning in your domain.

– Fluent interfaces are very useful though for builders. The Doctrine QueryBuilder or the PHPUnit MockBuilder for example would be very annoying without fluent interfaces.

Christian wrote at 3/12/2013 4:18 pm:

ouch.. all points already covered.. and you want to be my latex salesman?

Tjorriemorrie (@Tjorriemorrie) wrote at 3/13/2013 3:29 am:

Would it be viable to use trigger_error in some places instead of throwing an exception?

Sven (@SvenRtbg) wrote at 3/13/2013 7:05 am:

I cannot resist commenting, because “someone is wrong on the internet”. :)

1. “A return value can be used to tell if the function behaved properly.”

Well, yes, but usually a return value is used to return the result of a function. And it is up to the developer to decide which value should be returned if something went wrong. PHP functions usually return FALSE instead of something like a resource or integer values, and it can be hard to differentiate (strpos() == false is wrong).

On the other hand, if it isn’t a function, then it has no return value. These things used to be called procedures in other languages.

2. “A null return value can’t easily be tested.”

Wrong.

if (is_null($returnValue)) { /* failure code */ }
if (isset($returnValue)) { /* works as well */ }

$this->assertNull($returnValue); // unit testing

Testing for NULL is as easy as testing for FALSE. If the developer decides that instead of a usable value the function will return NULL in case of errors, this can easily be used.

3. “Returning a null value breaks fluent interfaces.”

This is no valid argument because any method inside fluent interfaces always must return it’s own object again. There simply is no decision whether or not NULL is the return value for failure. Reporting errors in OOP usually is done by throwing exceptions. Reporting emptiness is done by using empty or nonfunctional objects of the same interface.

I think this article needs more updating. The last part on what to return is ok, but the introductory discussion is misleading and presents the wrong arguments.

The question is: “What is a proper return value in case there is no obvious return value?” And the cases are a) dealing with errors, b) dealing with emptiness and c) default behavior when unconfigured.

Devon H. O'Dell (@dhobsd) wrote at 3/13/2013 4:54 pm:

I’d only point out that exceptions were not created to detect errors and should not be used for error handling. They were created to abort in unrecoverable situations. Using exceptions as an error-handling mechanism can be horribly error prone, especially in a language that does not enforce catching exceptions (and PHP does not).

Case in point, in case you’ve not read this yet:

http://blogs.msdn.com/b/oldnewthing/archive/2004/04/22/118161.aspx
http://blogs.msdn.com/b/oldnewthing/archive/2005/01/14/352949.aspx

Exceptions do not help your method chaining example. If I call $inst->foo()->bar()->bat()->baz(), and I catch an exception, where did it occur? I have to do a significant amount of work reading the data held in the exception instance to find out what failed. (This is one reason I really don’t like methods returning instances, *especially* when those methods might throw an exception.)

In fact, the only times I’ve heard people complain about return values from functions is because they don’t want to do error handling. This crops up on the Go mailing list every so often: people get unexpected behavior, they paste their code, and they handle no errors. In fact, I find the Go error handling solution rather elegant: http://blog.golang.org/2011/07/error-handling-and-go.html

Fundamentally, if you are not following the documented API and checking for possible errors, you deserve what you get. If a function or method can return null, don’t complain when your code breaks because you didn’t check for it. That’s *your* fault.

Hikari wrote at 4/9/2013 9:45 pm:

I know you already said this article is wrong and I’m commenting before reading the other article.

But SRSLY, a function return should NEVER be used to test if it worked in production code. For that, we have Exceptions!!

In the same way a function consumer may not know if a bad developed function worked if it has no return value, a bad developer may also not bother to test it and use the “error sign value” as if workable data. With Exception we assure consumer will know that something bad happened and there’s no returned data to use, or that his function will also return with the same Exception.

If you expect to do something and it fails, and you know that (an expected error, handled by the software!), and it can’t be bypassed, just throw a meaningful Exception and document it. PHP is bad on handling Exceptions because it doesn’t allow functions to enforce exceptions they may throw and it’s not compiled, therefore we lose a valuable moment to be warned if we don’t catch or rethrow them.

Recently I had to work on a software that was developed by outsource team. It was total junk, even though developed in Java+Servlet, it was full of errors and all Exceptions were being presented by Tomcat to user. It’s VERY frustrating when noobs let unhandled Exceptions show unformatted to users.

Copyright © 2023 by Brandon Savage. All rights reserved.