2

i have methods in a php class which returns boolean response the code i have used is.

public function checkIfExist($table ,$key, $value)
{
    $sth = $this->dbh->prepare("SELECT COUNT(*) FROM $table WHERE $key = :value");
    $sth->bindParam(':value', $value);
    $sth->execute();
    if($sth->fetchColumn() >= 1)
    {   
        return true;
    }
        return false;
}

the above method works for me without the else condition being included, logically it should work because once the functions get the true as boolean response it will exit the method. but is it the right way or should i be including else condition there?

7
  • 6
    return $sth->fetchColumn() >= 1; that's how I like em. Commented Apr 4, 2011 at 17:40
  • 1
    3 people beat me to it, just return the result of the comparison. Commented Apr 4, 2011 at 17:41
  • @mhitza and upvoters: it is a big questionmark, whether or not this way of writing this is better, it is definitely less readable and likely less maintainable. Commented Apr 4, 2011 at 17:49
  • @markus people still debate about which bracket styles are less/more readable, I'd rather not go there :) Commented Apr 4, 2011 at 17:51
  • @mhitza not about brackets, brackets have nothing to do with maintainability of code. Commented Apr 4, 2011 at 17:53

8 Answers 8

5

This is a perfectly fine way to write this logic. If you return from an if body, you can definitely omit the else.

It would be more concise, however, to just do this:

return $sth->fetchColumn() >= 1;
Sign up to request clarification or add additional context in comments.

4 Comments

it looks neat and readable and i think better then what i have used, thank you :)
@markus: do you write if statements like this? if ((a > b) == true) {...} Or like this? if (a > b) {...}
The second, of course. And if the first, then if (true == (a > b)) and not vice versa. But we're talking something else here!
The if statement has an 'if' which gives the semantic information which the notation we're talking about lacks.
2

Better to have your return like this:

return ($sth->fetchColumn() >= 1);

instead of if and else

1 Comment

Not necessarily better. Just more compact. Because if he wants later to add one single instruction before that return true he'll have to change all of it.
2

That's fine coding.

You don't need the else.

this is because if anything in that if statement was true --> you are returning, and if it is not true --> you are returning.

So the function is correct in its implementation.

Comments

1

This is fine since as you said the method returns already. Since your condition already returns a boolean you might also write

public function checkIfExist($table ,$key, $value)
{
    $sth = $this->dbh->prepare("SELECT COUNT(*) FROM $table WHERE $key = :value");
    $sth->bindParam(':value', $value);
    $sth->execute();
    return $sth->fetchColumn() >= 1;
}

Comments

1

Just return the result of the comparison:

public function checkIfExist($table ,$key, $value)
{
    $sth = $this->dbh->prepare("SELECT COUNT(*) FROM $table WHERE $key = :value");
    $sth->bindParam(':value', $value);
    $sth->execute();
    return ($sth->fetchColumn() >= 1);
}

Comments

0

It is really a matter of preferences.

Personally I like having only one return statement per function/method. This would require the use of a variable to hold the return value - but hopefully the gain is more readable code.

Comments

0

It is right. And also very practiced.

Comments

0

I use this kind of coding a lot:

function do_something($arg)
{
    if(some_condition_fails($arg))
    {
        return false;
    }

    if(some_other_condition_fails($arg))
    {
        return false;
    }

    //do it
    return $result;
}

There's no need to nest everything in else's...in fact, adding extra blocks around code that doesn't need it is dirtier (in my opinion) than leaving it out.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.