CODEWITHSUNDEEP
phpperformancereturn-valuedry
fabrik
fabrik

Reputation: 14365

When to return values in PHP?

Given this ugly method:

public function convert_cell_value( $val, $type )
{
    if($type == 'String')
    {
        return $val;
    }
    elseif($type == 'Number')
    {
        if($val - intval($val) > 0)
        {
            return $val;
        }
        else
        {
            return intval($val);
        }
    }
    else
    {
        return $val;
    }
}

Now my ten billion $ question is: when i should return values (not in this method but any other like this) to apply the DRY principles and go for performance too. Or: There's something wrong with my thought about performance and it's nothing to do with it when i return some value immediately?

Bonus question: Is there a simpler trick to get decimals than this?

if($val - intval($val) > 0)
{
    return $val;
}

Thanks for your precious time, fabrik

Upvotes: 1

Views: 141

Answers (6)

gianebao
gianebao

Reputation: 18908

Use ternary and implicit cast.

public function convert_cell_value($val, $type) {
    return $type === 'Number' && !is_float($val + 1) ? intval($val): $val;
}

Also, if your checking if the value has decimal, use is_float instead. intval produces a strict integer value. So if you have the value below and compare, the result would be false even though it should be true.

intval('420000000000000000000'); // 2147483647

Upvotes: 1

Victor Nicollet
Victor Nicollet

Reputation: 24577

Your function here could be refactored as:

function convert_cell_value($val, $type)
{
  if ($type == 'Number') 
    return intval($val);
  else
    return $val;
}

In practice, returning values is seldom subject to DRY since "return" is a minor redundancy that can usually only be replaced by assigning to a variable several times and returning that variable once.

What could be an argument against having multiple return statements is SESE (single entry single exit) which states there should only be one return statement, for readability reasons (you might miss one) and cleanup reasons (you have to clean up any allocated resources before you return).

In a situation like yours, your function structure is effectively "decide what to return", so the readability argument does not apply.

Upvotes: 2

enricog
enricog

Reputation: 4273

It depends on the function. If you have a function which does some different (and heavy performance cost) checks and the return value should be false if one check fails, it makes sense to return it immediately.

If it is just some "if (condition1) else " or a simple switch i would just return it in the end due to better readability.

Upvotes: 0

Gumbo
Gumbo

Reputation: 655189

You can simplify your method logic to this:

public function convert_cell_value( $val, $type )
{
    if ($type === 'Number' && ($ret = intval($val)) == $val) {
        return $ret;
    }
    return $val;
}

Or if you want to add more types, use a switch:

public function convert_cell_value( $val, $type )
{
    switch ($type) {
    case 'Number':
        if (($ret = intval($val)) == $val) {
            return $ret;
        }
    case 'String':
    default:
        return $val;
    }
}

You could also use just one return and replace return $ret by $val = $ret to return the right value.

Upvotes: 6

Elzo Valugi
Elzo Valugi

Reputation: 27856

As a rule, I prefer to have only 1 return per function for readability. Having a variable that is returned is 99,99% as fast as returning directly and is much more simple to debug.

// code from Gumbo
public function convert_cell_value( $val, $type )
{
    if ($type === 'Number' && ($ret = intval($val)) == $val) {
        $val = $ret; // this is the extra assigment
    }
    return $val; // only 1 return
}

Upvotes: 0

PJP
PJP

Reputation: 766

As your code will go only through one option, I would store the return value in a variable and return it at the end of the method.

Upvotes: 1

Related Questions