vick
vick

Reputation: 957

cleaner php code

if (isset($_GET['sort_by']) && strlen($_GET['sort_by']) > 0)
{
    $sort_by = $_GET['sort_by'];
}
else
{
     $sort_by = 'desc';  
}

how can I rewrite that so it looks cleaner and has less lines.. I love one liners hehe

Upvotes: 0

Views: 371

Answers (4)

Matt
Matt

Reputation: 112

I thought I would add that one-liners does not automatically mean cleaner code, however Yacoby did a great job of cleaning up the code (as well as making it a one liner). However I would tidy up his one liner to read....

$sort_by = empty($_GET['sort_by']) ? 'desc' : $_GET['sort_by'];

Simply because the ! symbol can sometimes be missed by someone reading the code - I think this is a clearer one liner in my opinion :)

Upvotes: 0

Galen
Galen

Reputation: 30170

$sort_by = in_array( $_GET['sort_by'], array( 'asc', 'desc' ) ) ? $_GET['sort_by'] : 'desc';

this also checks the vailidity

Upvotes: 1

Yacoby
Yacoby

Reputation: 55445

The first option is just to move the default value outside the if block and thereby remove the else. If you want to keep the conditions in the if statement this (probably) the best as it retains the clarity that is lost if you more it into a statment using the conditional (ternary) operator.

$sort_by = 'desc'
if (isset($_GET['sort_by']) && strlen($_GET['sort_by']) > 0){
    $sort_by = $_GET['sort_by'];
}

Although as strlen won't return less than 0, you can remove the > 0

$sort_by = 'desc'
if (isset($_GET['sort_by']) && strlen($_GET['sort_by']) ){
    $sort_by = $_GET['sort_by'];
}

Another option is to move everything onto one line and use the conditional operator. The downside is that it takes terseness (In my option) far to far and starts to put too much one one line.

$sort_by =  isset($_GET['sort_by']) && strlen($_GET['sort_by']) ? $_GET['sort_by'] : 'desc';

If you changed the functionality slightly so that you could use empty() the conditional operator becomes usable due to the reduced length. The issue is that the string "0" is treated as being empty.

$sort_by =  !empty($_GET['sort_by']) ? $_GET['sort_by'] : 'desc';

Upvotes: 5

Rich
Rich

Reputation: 36806

regardless of how you rewrite it (and Yacoby's got a good one-liner for you), if you do this often (null and strlen check query string vars), you should probably wrap it in a function with an option "default value" parameter).

ie. function getQueryStringVar($key, $defaultValue = '') { ...

Upvotes: 0

Related Questions