4
\$\begingroup\$

Consider this code segment:

if (empty($_GET)) {
    $page_number = 1;
} else {
    $page_number = $_GET["page_number"];
}

I wonder if there is a better, more succinct way to express that idea. I thought about using a ternary operator, but the improvement over the original is not substantial:

$page_number = empty($_GET) ? 1 : $_GET["page_number"];
\$\endgroup\$
0
2
\$\begingroup\$

EBrown has a good answer already, where he shows which steps has to be taken to make sure the page number is set correctly. He also comes up with a handy function. Let me show you another approach, using build-in functionality in PHP to handle the given problem.


Filter the input

Make use of PHPs filter_input:

$page_number = filter_input(INPUT_GET, 'page_number', FILTER_VALIDATE_INT);

It uses a predefined validation filter called FILTER_VALIDATE_INT, that …

… validates value as integer, optionally from the specified range, and converts to int on success.

So $page_number will be of type int and not string in case of success. If the filter fails, $page_name will be null|false:

Value of the requested variable on success, FALSE if the filter fails, or NULL if the variable_name variable is not set. If the flag FILTER_NULL_ON_FAILURE is used, it returns FALSE if the variable is not set and NULL if the filter fails.


Should I always default to 1?

Like in your question, if $page_number is not set at all, you should set it to 1. Using filter_input as described before you can test this case like:

if (null === $page_number) {
    $page_number = 1;
}

But there are more cases to be handled like:

  • invalid values, like -1|0|10000001
  • other non-integer values, like hello-world|a4fe|1.2

I recommend to not to default to 1 in any of these cases, but to show a 404 error page instead.

To make sure the integer value of $page_number will produce a valid page, you can take it one step further and set a range for the filter. The complete example would look like

$page_number = filter_input(
    INPUT_GET,
    'page_number',
    FILTER_VALIDATE_INT,
    ['options' => ['min_range' => 1, 'max_range' => 104]]
);

if (false === $page_number) {
    // show 404 page
}

if (null === $page_number) {
    $page_number = 1;
}

1 Unless you have 1000000 or more pages of course.

\$\endgroup\$
1
  • \$\begingroup\$ This is just what the doctor ordered. I did not know such a function exist. \$\endgroup\$ – Hai Vu Mar 6 '17 at 17:32
6
\$\begingroup\$

You have a surprise bug: what happens when I do page.php?Name=EBrown? Well $_GET is not empty but your page_number sure isn't set.

That said, you should be doing something like the following:

$page_number = 1;

if (isset($_GET["page_number"})) {
    $page_number = $_GET["page_number"];
}

or

$page_number = $_GET["page_number"];

if (!isset($page_number)) {
    $page_number = 1;
}

The first defaults and then checks for the parameter, the second checks if it's set then defaults. Take your pick.

But those still don't handle invalid (non-numeric) input, so we can do that as well:

$page_number = $_GET["page_number"];

if (isset($page_number) === FALSE || is_numeric($page_number) === FALSE) {
    $page_number = 1;
}

Now we safely default to 1 when page_number is not set, or when it's not numeric.

Of course, we still have another bug: what happens if page_number is 1.12? That's probably unintended. So, we'll check for that as well:

$page_number = $_GET["page_number"];

if (isset($page_number) === FALSE || strpos($page_number, '.') !== FALSE || is_numeric($page_number) === FALSE) {
    $page_number = 1;
}

And finally we'd extract that to a method:

function defaultInt($intValue, $default) {
    if (isset($intValue) === FALSE
        || strpos($intValue, '.') !== FALSE
        || is_numeric($intValue) === FALSE) {
        return $default;
    }

    return $intValue;
}

$page_number = defaultInt($_GET["page_number"], 1);

Easy to use in the future as well.

\$\endgroup\$
2
  • \$\begingroup\$ +1 good advice on better way to approach this, however I would also point out to OP that filter_var or filter_input might be a more concise way o validate input such as this. I would also consider casting the input to integer to be consistent across both cases. \$\endgroup\$ – Mike Brant Feb 26 '17 at 4:42
  • \$\begingroup\$ just for the record . page?page_number=%2B5 and page?page_number=-5 will pass your function ., page?page_number=0x5 will pass too if you are before php 7.0.0 . although a string variable equals "+5" or "-5" will be evaluated as integer in a mathematical context in loosely typed language like php . but this is might not be your intended behavior of the function. \$\endgroup\$ – Accountant م Feb 28 '17 at 5:09
0
\$\begingroup\$

if you want to try the shorthand of that equation, you can go for
$pageNumber = isset($_GET['page_number']) && intval($_GET['page_number']) > 0 ? $_GET['page_number'] : 1
This will check, if the page_number is set or not. If set then is the value greater than 0 then get the page number, otherwise set to 1.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.