I'm new to PHP and working on script that may have undefined variable and I have two ways to handle this, my question is what is the best way.
Variable $_GET['page']
can be sometimes defined and sometimes undefined.
the first way I check with isset() and if:
if(isset($_GET['page'])){
$sql.=limitrows($_GET['page'],10);
}else{
$sql.=limitrows(1,10);
}
the second way I just write the function and using @
remove the error and handle the isset()
inside the function
function limitrows($page=1, $rows=10){
if (isset($page)==false or $page<=0 ){
$page=1;
}
}
$sql.=@limitrows($_GET['page'],10);
the second way make my code much cleaner and I do all the dirty work inside the function but it show error and I have to @
. What is the best way?
Another approach is to use array_key_exists
to check for the existence of an array key. You can use an if
statement, or shorten it with a ternary.
// with if
$page = 1;
if (array_key_exists('page', $_GET) === true) {
$page = $_GET['page'];
}
// with ternary
$page = array_key_exists('page', $_GET) === true ? $_GET['page'] : 1;
Do not use the @
error suppression operator. It is a bad practice to get into, 99.99% of the time you want to see all errors in all situations.
The "best way" in coding is a combination of opinion, established code style, and industry best practices. Generally speaking, one should use the right functions for the job (in this case, array_key_exists
or filter_input
as indicated in another answer), not use error handling, suppression, or exceptions for normal code flow, and be explicit/verbose rather than trying for the shortest code. You might be able to condense something into 1 line rather than 3, but to what end? All it does is increase the specific complexity of any given part of your code, making it harder for others (and yourself!) to understand and maintain in the future.
In terms of where to do what, gather your arguments outside the function AND validate the arguments inside the function.
Documentation/More Reading
array_key_exists
- http://php.net/manual/en/function.array-key-exists.phpUse the ternary operator
$sql.=@limitrows(isset($_GET['page'])?$_Get['page']:1,10);
format [logic]?[true]:[false]
One possibility is to use a filter function to access the GET parameter.
$page = filter_input(INPUT_GET, 'page', FILTER_SANITIZE_NUMBER_INT);
With this approach, $page
will always be set, so you won't need to worry about undefined index warnings. (If 'page'
is not a key in $_GET
, filter_input
will return null
.)
Then, when you need to use $page
later, checking it is simpler:
function limitrows($page, $rows=10) {
if (!$page) $page = 1; // assuming you don't have a page 0, or
// if ($page === null) $page = 1; // if you DO have a page 0
// ...
}