0

I have nested if conditions on my project but i have a problem;

if(isset($_GET['q']) && isset($_GET['t'])) {
  $query = $_GET['q'];
  $type = $_GET['t'];
  $none_key = FALSE;

  if($type = 'singer') {
      $singers = $connect->query("SELECT * FROM lyrica_singers WHERE singer_name LIKE '%$query%'");
      $control = $singers->rowCount();

      if($control > 0)  {
        $on_page = 24;
        $number_singers = $singers->rowCount();
        $number_page = ceil($number_singers/$on_page);

        $page = isset($_GET['p']) ? (int) $_GET['p'] : 1;
        if ($page < 1) $page = 1;
        if ($page>$number_page) $page = $number_page;

        $limit = ($page - 1) * $on_page;

        $singers = $connect->query("SELECT * FROM lyrica_singers WHERE singer_name LIKE '%$query%' ORDER BY singer_name ASC LIMIT ".$limit.",".$on_page);
        $singer_key = TRUE;
      } else {
        $none_key = TRUE;
      }
    }

    if($type = 'song') {
      $songs = $connect->query("SELECT * FROM lyrica_songs WHERE song_name LIKE '%$query%'");
      $control = $songs->rowCount();

      if($control > 0)  {
        $on_page = 24;
        $number_songs = $songs->rowCount();
        $number_page = ceil($number_songs/$on_page);

        $page = isset($_GET['p']) ? (int) $_GET['p'] : 1;
        if ($page < 1) $page = 1;
        if ($page>$number_page) $page = $number_page;

        $limit = ($page - 1) * $on_page;

        $songs = $connect->query("SELECT * FROM lyrica_songs WHERE song_name LIKE '%$query%' ORDER BY song_name ASC LIMIT ".$limit.",".$on_page);
        $song_key = TRUE;
      } else {
        $none_key = TRUE;
      }
    }
} else {
  $key = TRUE;
}

When I run the code, I am expecting that if one of the'control' variables is bigger than 0 the 'none_key' variable must be equal to 0. When the 'type' varible is 'song' there is no problem but if the 'type' variable is 'singer' then 'none_key' variable printing 1, I think it is running the second if block and becomes 'none_key' 1 because of the second 'control' variable is not bigger than zero.

  • 2
    Possible duplicate of [The 3 different equals](https://stackoverflow.com/questions/2063480/the-3-different-equals) – Qirel Aug 02 '18 at 20:43
  • 1
    **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation or concatenation to accomplish this because you have created a severe [SQL injection bug](http://bobby-tables.com/). **NEVER** put `$_POST`, `$_GET` or **any** user data directly into a query, it can be very harmful if someone seeks to exploit your mistake. – tadman Aug 02 '18 at 20:46

1 Answers1

3

This is an assignment, it sets $type to the value singer:

if ($type = 'singer') {

You want a comparison, it checks to see if $type equals the value singer:

if ($type == 'singer') {

[Edit] Some people prefer to write this sort of statement with the variable last. This is commonly called a "Yoda condition":

if ('singer' == $type) {

This way, if you screw up and only use one =, you get an error.

Alex Howansky
  • 50,515
  • 8
  • 78
  • 98
  • Another possible fix is to use a [`switch`](http://php.net/manual/en/control-structures.switch.php) statement where assignment is not going to happen by accident. – tadman Aug 02 '18 at 20:47
  • @tadman That seems unnecessary - at least 2-3 extra lines of code so you don't accidentally forget a single `=`? – GrumpyCrouton Aug 02 '18 at 20:48
  • @GrumpyCrouton It's a good way to handle multiple cases without piles of `if` statements. I don't mean for a single `if`, I mean for this specific situation where there's two comparisons, potentially more later. – tadman Aug 02 '18 at 20:49
  • @tadman Well that's definitely true, but with that said OP's code could easily be simplified to a much shorter amount of code (and still be readable, scalable) because there is a ton of code repetition. – GrumpyCrouton Aug 02 '18 at 20:49
  • I've been coding for hours and I really missed it, thank you so much. – cem ekkazan Aug 02 '18 at 20:56
  • Huh, I've never heard/seen Yoda Condition, but it does make sense, very interesting :) – GrumpyCrouton Aug 02 '18 at 20:58
  • 1
    Heh, I don't use them, they annoy me. I just use a PHPCS sniff to disallow embedded assignments. – Alex Howansky Aug 02 '18 at 21:04
  • I used to use Netbeans and it complained about assignment in conditionals if I remember correctly – Don't Panic Aug 02 '18 at 21:11