Samuel Hughes Mensah
Samuel Hughes Mensah

Reputation: 65

Using if and else statements error

I have created two links where I would like the page contents to change. The problem is the URL changes but not the page content.

<h3>Filter Results</h3>
<p><a href="index.php?filter='Action'>Action</a></p>
<p><a href="index.php?filter='Comedy'">Comedy</a></p>
if (isset($_GET['filter']) == 'Action') {
    echo 'Action';  
}
else if (isset($_GET['filter']) =='Comedy') {    
    echo 'Comedy';
}

It always outputs the first link information "Action".

Upvotes: 3

Views: 187

Answers (10)

Joem Maranan
Joem Maranan

Reputation: 150

Make sure that you insert an opening and a closing php tag: <?php and ?> To simplify it a bit you could just echo the value you get via $_GET

<h3>Filter Results</h3>
<p><a href="index.php?filter='Action'>Action</a></p>
<p><a href="index.php?filter='Comedy'>Comedy</a></p>
<?php
    if(isset($_GET['filter'])){
        echo $_GET['filter'];  
    }
?>

Upvotes: 3

Madara&#39;s Ghost
Madara&#39;s Ghost

Reputation: 174947

Your links are faulty:

<p><a href="index.php?filter=Action">Action</a></p>
<p><a href="index.php?filter=Comedy">Comedy</a></p>
<!--                         ^    ^ No single quotes (' ') -->

Yogesh Suthar pointed it out first

Also, isset() will return a boolean (true or false; based on whether or not the variable is set). You're comparing a boolean to a string, a string will always be converted into TRUE (unless the string is "false" or similar), so basically, if the variable is set, the first condition will always match.

You want

if (isset($_GET["filter"]) && $_GET["filter"] === "Action")

Note the use of ===, this will make sure that the variable is exactly what you think it is, and not some sort of other type variable.

Few more points (Shamelessly stolen taken from other answers)

  • If there are multiple possible filters, check for the variable existance once, and use a switch/case block to determine which of them it is:

    if(isset($_GET['filter'])) {
        switch($_GET['filter']) {
            case 'Action':
                echo 'Action';
                break;
            case 'Comedy':
                echo 'Comedy';
                break;
        }
    }
    

Upvotes: 7

Sundar
Sundar

Reputation: 4650

//isset will always return true or false
if(isset($_GET['filter'])){

    if($_GET['filter']=='Action')
    {
        echo 'Action';

    }elseif($_GET['filter']=='Comedy'){

        echo 'Comedy';

    }

}

Upvotes: 0

Lars Ebert-Harlaar
Lars Ebert-Harlaar

Reputation: 3537

The function isset will only check if the variable is existing! It will not return its value! Try this instead:

<h3>Filter Results</h3>
<p><a href="index.php?filter=Action">Action</a></p>
<p><a href="index.php?filter=Comedy">Comedy</a></p>
if(isset($_GET['filter']) && $_GET['filter'] == 'Action'){
    echo 'Action';  
}

else if(isset($_GET['filter']) && $_GET['filter'] == 'Comedy') {
    echo 'Comedy';
}

Also, using switch might make things easier in the future:

<h3>Filter Results</h3>
<p><a href="index.php?filter=Action">Action</a></p>
<p><a href="index.php?filter=Comedy">Comedy</a></p>
if(isset($_GET['filter'])) {
    switch($_GET['filter']) {
        case 'Action':
            echo 'Action';
            break;
        case 'Comedy':
            echo 'Comedy';
            break;
    }
}

Upvotes: 5

Macbric
Macbric

Reputation: 482

Wrong use of isset, check documentation, return a boolean.

if (isset($_GET['filter']))
{
    switch ($_GET['filter'])
    {
        case 'Action':
            //TODO
            break;
        case 'Comedy':
            // TODO
            break;
        default:
            // TODO
            break;
    }
}

Upvotes: 1

Powerslave
Powerslave

Reputation: 1428

isset returns true and since 'Action' is not null, it evaluates to true.

if ((isset($_GET['filter'])) && ($_GET['filter'] == 'Action')) {
    // ...
} else if ((isset($_GET['filter'])) && ($_GET['filter'] == 'Comedy')) {
    // ...
}

BTW such code would sooner or later become a nightmare to maintain.

You could instead, for example

function preventDirectoryTraversal($requestParam) {
    return preg_replace("/\//", "", $requestParam);
}

// ...

if (isset($_GET['filter'])) {
    $filterName = preventDirectoryTraversal($_GET['filter']);
    include(FILTERS_DIR . "/" .  $filterName . ".php");
}

or something alike. Of course this can be further improved, but I hope you get the point.

Upvotes: 1

Pieter
Pieter

Reputation: 1833

The function isset will return true or false (it checks whether the variable is set or not). Change your code:

if(isset($_GET['filter']) && $_GET['filter'] == 'Action') {

Upvotes: 2

Yogesh Suthar
Yogesh Suthar

Reputation: 30488

As @MadaraUchiha said about isset and,

if(isset($_GET['filter']) == 'Action')

should be

if(isset($_GET['filter']) && $_GET['filter'] == 'Action')

Also

<a href="index.php?filter='Action'>Action</a>
        ^                 ^      ^ // here you started " but not ended and remove the single quotes around Action

should be

<a href="index.php?filter=Action">Action</a>

Upvotes: 3

shark
shark

Reputation: 325

you dont have to use isset() and then compare.

$filter = $_GET['filter'];

if(isset($filter)){
   if($filter == 'Action'){
     echo 'Action';
   }else if($filter == 'Comedy'){
     echo 'Comedy';
   }

}

Upvotes: 1

Code Lღver
Code Lღver

Reputation: 15603

Your if condition is not correct do it:

if(isset($_GET['filter']) && $_GET['filter'] == 'Action'){
  echo 'Action';  
}

Similarly with else if:

else if(isset($_GET['filter']) && $_GET['filter'] =='Comedy') {

As you are comparing the isset($_GET['filter']) with the value although isset returns true of false so you need to compare the value of $_GET['filter'].

Upvotes: 1

Related Questions