I want to understand why this function doesn't return anything.
function fact($n, $p = 1) {
if ($n > 1) {
$p *= $n--;
fact($n, $p);
} else {
return $p;
}
}
var_dump(fact(5)); // NULL
You try to assign a variable which you did not pass by reference. Either pass $p by reference (&$p
) or use a return value. In this case a return value is way better.
Secondly, $n--
is post decrement, meaning your code does not read that nicely.
function fact($n) {
if ($n == 0) return 1;
return $n * fact($n - 1);
}
var_dump(fact(5))
Recursion is a looping construct which comes from functional languages. So yes, as others noted, your function doesn't work correctly because the true branch of your
if
statement doesn't return anything. However, I have additional remarks about your codeYou're actually mutating two variables here in one expression. It's clever if you're trying to keep the code looking shorter, but there's actually an even better way.
Now we don't have to think about how
$p
and$n
change individually. We just know that we callfact
again with the next values for each state of$p
and$n
.Keep in mind, these principles are so strong in some functional programming languages that reassignment of variables likes
$p
and$n
is not even allowed.Lastly, we have to talk about your your API leak,
$p
. If someone were to specify a value when callingfact
, they could get the wrong answer or trigger an errorThis is only possible because
$p
is actually exposed in the public API. To get around this, you have a couple of optionsOne of them is to do as @RonaldSwets proposes:
Another is to use an auxiliary function which is meant only for private use