problem with an IF statement in a function

Question

I am novice PHP programmer and I have created a function that changes an SQL command. Here is relevant piece of code:

 $extra_text_length = strlen($_GET[extra_text]);
 $boolean = $_GET['first-boolean'];

 function check_boolean(){

  if($extra_text_length > 0){

    if($boolean=="and"){
        $query .= " AND (software.SWID='$_GET[extra_text]')";
        } elseif($boolean=="or"){
        $query .= " OR (software.SWID='$_GET[extra_text]')";
        } elseif($boolean=="not"){
        $query .= " NOT IN (software.SWID='$_GET[extra_text]')";
        }
    } return $boolean;
  }

check_boolean();

The problem is that it doesn't do what it should do. If I remove the code from the function however and as consequence I remove the check_boolean() method it works perfectly. Would anyone give me a hint? Thanks in advance

Wiseguy · Accepted Answer · 2011-05-03 19:19:09Z

3

boolean is missing its $, though I'd prefer to make it a parameter like function check_boolean($bool) instead of using a global var. Also, please read up on SQL injection. It's a terrible vulnerability you have right now. Shortest solution is to put mysql_real_escape_string() around the vars in your query (assuming you're using MySQL), but please learn about SQL injection thoroughly.

edited May 3, 2011 at 19:19

answered May 3, 2011 at 19:12

Wiseguy

20.9k9 gold badges68 silver badges83 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

jsgoupil · Accepted Answer · 2011-05-03 22:14:39Z

Make sure to use the variable name $boolean instead of boolean, otherwise it's trying to get a constant.

You also have to be careful to what you pass to your method. $query is a variable that you haven't defined either.

Make sure to do something like this

 $extra_text_length = strlen($_GET[extra_text]);
 $boolean = $_GET['first-boolean'];

 function check_boolean($query, $extra_text_length, $boolean, $software){

  if($extra_text_length > 0){

    if($boolean=="and"){
        $query .= " AND ($software.SWID='$_GET[extra_text]')";
        } elseif($boolean=="or"){
        $query .= " OR ($software.SWID='$_GET[extra_text]')";
        } elseif($boolean=="not"){
        $query .= " NOT IN ($software.SWID='$_GET[extra_text]')";
        }
    }
    return $query;
  }

$query = '...';
$software = '';
$query = check_boolean($query, $extra_text_length, $boolean, $software);

Also, avoid using external variables in a function, since you don't know how they will react. ($_GET) You should also check the existence of all your variables.

Sony Santos · Accepted Answer · 2011-05-03 19:30:55Z

First you must use $boolean instead of boolean inside if expressions.

Second: $query, $extra_text_length, and $boolean must be visible inside check_boolean. You can do it by passing them as parameters, e.g.:

function check_bookean(&$query, $extra_text_length, $boolean) {
    # ...
}

check_boolean($query, $extra_text_length, $boolean);

$query needs to be prepend by an ampersand (&) in the function definition because it's modified inside the function.

check_boolean will then take you query and add a boolean condition to it, based upon extra_text. But you need sanitize the content before including it in the query, to avoid SQL injection:

if($extra_text_length > 0){

    $extra_text = mysql_real_escape_string($_GET['extra_text']);  # sanitize content

    if(boolean=="and"){
        $query .= " AND (software.SWID='$extra_text')";           # to use it in the query
        } elseif(boolean=="or"){
        $query .= " OR (software.SWID='$extra_text')";
        } elseif(boolean=="not"){
        $query .= " NOT IN (software.SWID='$extra_text')";
        }
    } return boolean;
}

stivlo · Accepted Answer · 2011-05-05 10:06:55Z

1

Well the problem is the scope of the variables, when you enter in the function block, the variables in the global scope are not visible.

You should pass them as parameter. Your call should be:

check_boolean($extra_text_length, $boolean, $software);

and you've to change your function declaration as:

function check_boolean($extra_text_length, $boolean, $software)

Also, it would be better to avoid mixing the global code and your functions. you could put them in a separate file and use require_once to include them.

I would also encourage you not to use any global code at all. for example you can wrap the first two lines in a main() function.

Also "return boolean;" should be "return $query;"

edited May 5, 2011 at 10:06

answered May 3, 2011 at 19:12

stivlo

85.9k35 gold badges148 silver badges200 bronze badges

Comments

martinstoeckli · Accepted Answer · 2011-05-03 19:12:08Z

0

You forgot the $ sign before $boolean. I did the same when i started with PHP knowing other languages ;-).

answered May 3, 2011 at 19:12

martinstoeckli

24.2k7 gold badges63 silver badges92 bronze badges

Comments

Chien-Wei Huang · Accepted Answer · 2011-05-03 19:12:19Z

0

boolean=="and" should be $boolean=="and"

answered May 3, 2011 at 19:12

Chien-Wei Huang

1,8611 gold badge18 silver badges28 bronze badges

Comments

Felix Kling · Accepted Answer · 2011-05-03 19:19:06Z

$extra_text_length, $boolean and $query are not available inside the function. You have to pass them as arguments.

In addition, changing an argument inside a function does not change its value outside the function. Return the text instead. Here is a slightly improved version:

function check_boolean($length, $b, $extra_text){
  $operators = array('and', 'or', 'not');
  if($length > 0){
     if(in_array($b, $operators){
         return " " . $b . " (software.SWID='" . $extra_text . "')";
     }
  }
  return '';
}

$query .= check_boolean($extra_text_length, $boolean, mysql_real_escape_string($_GET['extra_text']));

Note that you have to call mysql_real_escape_string on the user input, otherwise your code is prone to SQL injection.

Collectives™ on Stack Overflow

problem with an IF statement in a function

7 Answers 7

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related