CODEWITHSUNDEEP
php
user2312612
user2312612

Reputation:

PHP nesting if statements - is there a better way?

Could I nest these PHP IFs in a better to understand/ more logical way?

 if (isset($_GET['numberofwelds']) && isset($_GET['numberofconwelds']))
    {
        $numwelds = $_GET['numberofwelds'];
        $numconwelds = $_GET['numberofconwelds'];

        if (is_int($numwelds) && is_int($numconwelds))
        {
            $total = $numwelds + $numconwelds;
            $response = json_encode($total);
            header(“Content-Type:application/json”);  
            echo $response;
            exit;
        }
    }

Upvotes: 0

Views: 54

Answers (4)

mareckmareck
mareckmareck

Reputation: 1580

First of all, you could invert conditionals like many IDEs (for example PHPStorm) tell you to, so you could do it like:

if (!isset($_GET['numberofwelds']) || !isset($_GET['numberofconwelds'])) {
    exit;
}

$numwelds = $_GET['numberofwelds'];
$numconwelds = $_GET['numberofconwelds'];

if (!is_int($numwelds) || !is_int($numconwelds)) {
    exit;
}

$total = $numwelds + $numconwelds;
$response = json_encode($total);
header("Content-Type:application/json");  
echo $response;

As you can see, the nesting was almost eliminated. However, I encourage to use more functional (or object) way of doing things. Simple example:

if (checkGetData($numwelds, $numconwelds)) {
    $total = $numwelds + $numconwelds;
    $response = json_encode($total);
    header("Content-Type:application/json");  
    echo $response;
    exit;
}

function checkGetData($numwelds, $numconwelds) {
    if (isSetAndNumeric('numberofwelds') 
         && isSetAndNumeric('numberofconwelds')) {
        $numwelds = $_GET['numberofwelds'];
        $numconwelds = $_GET['numberofconwelds'];
        return true;
    }
    return false;
}

function isSetAndNumeric($property) {
    if (isset($_GET[$property]) && is_numeric($_GET[$property])) {
        return true;
    }
    return false;
}

Writing code in such way is much more readable, especially as projects get bigger and more complicated.

Upvotes: 0

Gil
Gil

Reputation: 1804

You can do the checking in the first if. The first condition check if both fields exist and the second one check for the type.

if ( (isset($_GET['numberofwelds']) && isset($_GET['numberofconwelds'])) &&
     (is_int($_GET['numberofwelds']) && is_int($_GET['numberofconwelds']) )
{
    $numwelds = $_GET['numberofwelds'];
    $numconwelds = $_GET['numberofconwelds'];

    $total = $numwelds + $numconwelds;
    $response = json_encode($total);
    header(“Content-Type:application/json”);  
    echo $response;
    exit;
}

Upvotes: 1

Amal Murali
Amal Murali

Reputation: 76636

You can use ternary statements to make this a little bit nicer. The logic is the same, but you can reduce the number of lines required, at the expense of a little readability.

Some other important points to note:

  • You had smart quotes. “ is not the same as ". The former will not be parsed by PHP -- and will most likely throw an Internal Server Error.

  • Don't use is_int() — it will lie to you. When you're working with user input data, you'll be fetching it from $_POST, $_GET superglobal arrays. All those values will be stored as strings. is_int() treats a string containing a number, for e.g., "42" as as a string, and will return false. Use is_numeric() instead.

Updated code:

$numwelds = isset($_GET['numberofwelds']) ? $_GET['numberofwelds'] : '';
$numconwelds = isset($_GET['numberofconwelds']) ? $_GET['numberofconwelds'] : '';

if (is_numeric($numwelds) && is_numeric($numconwelds))
{
    $total = $numwelds + $numconwelds;
    $response = json_encode($total);
    header("Content-Type: application/json");  
    echo $response;
    exit;
}

Upvotes: 1

user3393647
user3393647

Reputation: 1

Im not an "@" fan but maybe 

    if($numwelds=@$_GET['numberofwelds'] && $numconwelds=@$_GET['numberofconwelds'] &&         is_int($numwelds) && is_int($numconwelds)) {
            $total = $numwelds + $numconwelds;
            $response = json_encode($total);
            header("Content-Type: application/json");  
            echo $response;
            exit;
    }

Upvotes: -1

Related Questions