What's wrong with my logic?

[yandoo]

Hello

I was hoping for some help with my if statement logic as I'm getting strange results and wonder if I am doing something (or lots) totally wrong.

 

I have a mysql query that pulls 3 planets(Anoat, Sullust, Umbara) from a table, the results are in a while loop and the 3 if else statements are within it. The idea is that by using the if statements I can assign css classes to display the planet names in different positions and style. This seemed to be working fine until I noticed a problem and now it's made me wonder if I'm going about this totally wrong.

The issue is that if the if condition (name, faction id and identified) all are met, not only is the correct output name displayed but the next planet name displays with it. For example SullustUmbra.I have no idea why it would do that as umbara has is own if else statement.

 

If you wouldn't mind putting me out my misery I'd be very grateful.

 

Thank you

Yan.

$query_planet = "SELECT * FROM planet ";
$planet = mysql_query($query_planet, $conn) or die(mysql_error());
$totalRows_planet = mysql_num_rows($planet);

while ($row_planet = mysql_fetch_array($planet)) 
{
   	
//Anoat                       
	  if (($row_planet['name'] == "Anoat") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '1')){ 
	  
	  echo '<a href="" class="anoat green">Anoat</a>';
	  
	  } else if (($row_planet['name'] == "Anoat") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '2')){ 
	  
	  echo '<a href="" class="anoat red">Anoat</a>';
	  
	 	
	 } else if (($row_planet['name'] == "Anoat") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '2'  ))   { 
	  echo '<span class="anoat red">Anoat</span>';
	 
	  } else if (($row_planet['name'] == "Anoat") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '1'  ))   { 
	  echo '<span class="anoat green">Anoat</span>';	 
	  
	   }  else if (($row_planet['name'] == "Anoat") && ($row_planet['factionid']  !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '0'))   { 
	  echo '<span class="Anoat">Anoat</span>';
	 
	
	 } else if (($row_planet['name'] == "Anoat") && ($row_planet['factionid'] ==  '0'))  { 
	  echo '<span class="anoat">Anoat</span>';
	 
	}
	
	
	
 
//Sullust
 if (($row_planet['name'] == "Sullust") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '1')){ 
	  
	  echo '<a href="" class="Sullust green">Sullust</a>';
	  
	 } else if (($row_planet['name'] == "Sullust") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '2')){ 
	  
	  echo '<a href="" class="Sullust red">Sullust</a>';
	  
	 	
	 } else if (($row_planet['name'] == "Sullust") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '2'  ))   { 
	  echo '<span class="Sullust red">Sullust</span';
	 
	  } else if (($row_planet['name'] == "Sullust") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '1'  ))   { 
	  echo '<span class="Sullust green">Sullust</span>';	 
	 
	 
	  }  else if (($row_planet['name'] == "Sullust") && ($row_planet['factionid']  !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '0'))   { 
	  echo '<span class="Sullust">Sullust</span>';
	 
	
	 }  else if (($row_planet['name'] == "Sullust") && ($row_planet['factionid'] ==  '0'))   { 
	  echo '<span class="Sullust">Sullust</span>';
	 
	}
 

//Umbara
 if (($row_planet['name'] == "Umbara") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '1')){ 
	  
	  echo '<a href="" class="Umbara green">Umbara</a>';
	  
	} else if (($row_planet['name'] == "Umbara") && ($row_planet['factionid'] ==  $_SESSION['MM_Username']) && ($row_planet['factionid'] == '2')){ 
	  
	  echo '<a href="" class="umbara red">Umbara</a>';
	
	 	
	 } else if (($row_planet['name'] == "Umbara") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '2'  ))   { 
	  echo '<span class="Umbara red">Umbara</span>';
	 
	  } else if (($row_planet['name'] == "Umbara") && ($row_planet['factionid'] !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '1') && ($row_planet['factionid'] == '1'  ))   { 
	  echo '<span class="Umbara green">Umbara</span>';	 
	  
	   }  else if (($row_planet['name'] == "Umbara") && ($row_planet['factionid']  !=  $_SESSION['MM_Username']) && ($row_planet['identified'] == '0'))   { 
	  echo '<span class="Umbara">Umbara</span>';
	 
	
	 	 } elseif (($row_planet['name'] == "Umbara") && ($row_planet['factionid'] ==  '0'))   { 
	  echo '<span class="umbara">Umbara</span>';
	 
	}
	
	
		}		

Edited August 11, 2017 by yandoo

[yandoo]

It was missing html > on the span. OK so I guess the logics OK.

Thank you.

[kicken]

You can reduce your code duplication quite a bit if you use a few extra variables and some convention/logic. I tried simplifying what you have above, but since I'm not too sure exactly what you're different variables mean/represent it's possible I may have missed something.

while ($row_planet = mysql_fetch_array($planet)){
    $identified = $row_planet['identified'] == '1';
    $factionMatches = $row_planet['factionid'] == $_SESSION['MM_Username'];
    $colorClass = '';
    if ($row_planet['factionid'] == '1'){
        $colorClass = 'green';
    } else if ($row_planet['factionid'] == '2'){
        $colorClass = 'red';
    }

    $name = $row_planet['name'];
    $nameClass = strtolower($name);
    if ($factionMatches){
        echo '<a href="" class="' . $nameClass . ' ' . $colorClass . '">' . $name . '</a>';
    } else if ($identified){
        echo '<span class="' . $nameClass . ' ' . $colorClass . '">' . $name . '</span>';
    } else {
        echo '<span class="' . $nameClass . '">' . $name . '</span>';
    }
}

First, the three separate blocks can be reduced to one by just using the name variable in the output rather than hard-coding it.

 

Second, your color highlight class appears to be dependent on just whether the factionid is 1 or 2, so that can be decided and stored into a variable.

 

Lastly your main output seems to be based on three main conditions:

• If the faction matches the session value, show a link
• Otherwise, if the faction has been identified, show a colored name
• Otherwise, show an uncolored name

Adjust the logic as necessary if I got something wrong. The main point here is avoid repeating your self in your code.

[Psycho]

Sorry to be blunt, but you really need to work on your logic skills. All those elseif() hurt to look at. You could have at least done something like this (abbreviated):

 

if($row_planet['name'] == "Anoat")
{
    if($row_planet['factionid'] == $_SESSION['MM_Username'])
    {
        //Additional code for creating anchor tag
    }
    else
    {
        //Additional code for creating span tag
    }
}

 

But, kicken's suggestion is the right one. I took a slightly different approach, but I think this will produce the same output as you originally had:

 

while ($row_planet = mysql_fetch_array($planet)) 
{
    //Determine if the element will be a link (or span)
    $isLink = ($row_planet['factionid'] == $_SESSION['MM_Username'] && in_array($row_planet['factionid'], [1,2]));
    //Determine the primary class for the element from the 'name'
    $primary_class = $row_planet['name'];
    //Determine secondary class for the element
    $secondary_class = ''; //Default value
    if($isLink || $row_planet['identified']=='1')
    {
        $secondary_class = ($row_planet['factionid'] == '1') ? 'green' : 'red';
    }
    //Create output
    if($isLink) {
        echo "<a href='' class='{$primary_class} {$secondary_class}'>Anoat</a>";
    } else {
        echo "<span class='{$primary_class} {$secondary_class}'>Anoat</span>";
    }
}

[benanamen]

What no one has mentioned is that your code is obsolete, dangerous and has been completely removed from Php. Also, you never ever output internal system errors to the user. That info is only good to hackers and useless to the user. You need to use PDO with prepared statements. https://phpdelusions.net/pdo

 

I realize the previous answers are providing responses to your specific question but using obsolete Mysql_* is never the right answer.