Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Something wrong with disregarding a returned value from a function in PHP

For instance:

// somefile.php
function doSomething() {
    // do lots of code, whatever...
    return $something;
}

// mainfile.php
include "somefile.php"
doSomething(); // ignored the return value, we don't need it here

What happens when a function in PHP returns a value but we don't care about it? Is something wrong with this behavior or should we always get the variable even if we'll never used it outside the function scope? How does PHP manage it's resources by returning a value that won't be used outside of the function scope?

like image 776
rfgamaral Avatar asked Sep 14 '10 13:09

rfgamaral


4 Answers

The return value is discarded. There is nothing wrong with doing that. Even functions without an explicit return do return null implicitly:

function foo() {}
var_dump(foo());  // NULL
like image 169
Gumbo Avatar answered Oct 21 '22 12:10

Gumbo


For me it all bowls down to a simple paradigm:

“A function should do one thing. It should do it well. It should do it only.”

If you have more than one reason to call that function is does to much.


From a language standpoint there is absolutely no problem with disregarding return values. From a clean code standpoint there is.

A insert() function should insert a record in a database. Not return the id of the inserted record. You didn't ask for that.

A setLatitude() should modify the objects internal state not load your Timezone class and figure in which timezone that latitude is and them write that to the database. (Saving the object can do that).


If you have two reasons to call a function it serves more than one purpose and from a clean code point of view it therefore: "Does too much"

Surely there can be cases constructed where it can make sense but as a general rule of thumb disregarding return values can be a code smell.

like image 28
edorian Avatar answered Oct 21 '22 12:10

edorian


It's code smell, every time you return something which is not going to be used means you don't have an idea about your audience - you might be missing some other concept.

Also, such functions usually suffer from Command Query separation.

Few examples:

  • array_unshift(&$arr, $value, ...) prepends passed values to array and returns new size of array, how often do you want that information? and if you do, you can always call count($array) which was designed for this purpose.

  • sort(&$arr) sorts passed array and returns if it was successful. does it make sense? when something wrong happened you should know, exception should be raised/error triggered, do you really want to test success all the time? and do you?

  • print_r() is uber-example of this, it returns true(always so it's useless anyway) or string, depending on arguments you pass into it. this is definitely not how api should look like.

One possible exception could be fluent interface, like jQuery but that is VERY specific case.

HINT: Write few tests and rethink your architecture, if it really doesn't make sense in any other way, it's probably good enough. But you should always be careful in such situations.

like image 2
Kamil Tomšík Avatar answered Oct 21 '22 10:10

Kamil Tomšík


There's nothing wrong with it. If you have a function that returns a value but also changes the program's state in some way (what's called a side effect), calling the function but ignoring the return value will simply cause all the side effects to be applied. You will, of course, not be able to use the return value if you don't store it, but sometimes all you want is the side effects of a call.

There are some people that believe in designing APIs in such a way that functions called for their side effects (commands) and functions called to observe some state of the program (queries) are separated as much as possible. This principle is called Command-Query Separation. The idea is that all functions that return something should not modify the program's state because they are observing it, and the act of observation should not affect the state observed. All functions that do not return anything are then seen as commands, called exclusively for their side effects.

People who follow these principles might therefore think that since your function clearly applies a side effect (or why would you call it and not care about the return value?), it should not observe any state (have a return value). Note, however, that this principle is not meant to always be followed to the letter, and there's also no huge agreement on it.

Why do some advocate command-query separation? Because it ensures that consecutive queries return the same answer if no command has been applied between the queries. It's sort of a weaker form of immutability, and carries a weakened form of the advantages that immutability can have for reasoning about the logic of a program. When applied consistently, objects are immutable in between commands.

In my view, keeping this principle in mind often results in less confusing APIs and clearer program logic, but it should not be carried too far. There are times when you may need to apply a side effect and return a value. When the return value is just a success value, however, consider raising an exception on failure instead.

like image 2
Gravity Avatar answered Oct 21 '22 10:10

Gravity