PHP multiple conditions for if statement that changes font color

Question

I've some problems with this PHP code:

 // HIGHLIGHT NUMBERS *************************************************
 // BUY
$valueEnlightOverBuy = $_POST["enlightOverBuy"];
$valueEnlightUnderBuy = $_POST["enlightUnderBuy"];

// GREEN PRICE BUY
$fontColorBuy = "#FFF"; 

    if ((($valueEnlightOverBuy != '') or ($valueEnlightUnderBuy != '')) and (($valueEnlightOverBuy and $valueEnlightUnderSell) != "0")) {
    if (($finalPriceBuyer >= $valueEnlightOverBuy) or ($finalPriceBuyer <= $valueEnlightUnderBuy)) {
    $fontColorBuy = "#00FF00";
    } else if (($finalPriceBuyer >= $valueEnlightOverBuy and $finalPriceBuyer <= $valueEnlightUnderBuy)) {
    $fontColorBuy = "#FF00FF";
    } else {
    $fontColorBuy = "#D00000";
    }};

 // SELL LOCAL
 $valueEnlightOverSloc = $_POST["enlightOverSloc"];
 $valueEnlightUnderSloc = $_POST["enlightUnderSloc"];

 // GREEN PRICE SELL LOCAL
 $fontColorSellLoc = "#FFF";    

    if ((($valueEnlightOverSloc != '') or ($valueEnlightUnderSloc != '')) & (($valueEnlightOverSloc & $valueEnlightUnderSloc) != "0")) {
    if (($finalPriceSellerLocal >= $valueEnlightOverSloc) or ($finalPriceSellerLocal <= $valueEnlightUnderSloc)) {
    $fontColorSellLoc = "#00FF00";
    } else if (($finalPriceSellerLocal >= $valueEnlightOverSloc) and ($finalPriceSellerLocal <= $valueEnlightUnderSloc)) {
    $fontColorSellLoc = "#FF00FF";
    } else {
    $fontColorSellLoc = "#D00000";
    }};

 // SELL INTERNATIONAL
 $valueEnlightOverSellInt = $_POST["enlightOverSellInt"];
 $valueEnlightUnderSellInt = $_POST["enlightUnderSellInt"];

 // GREEN PRICE SELL INTERNATIONAL
 $fontColorSellInt = "#FFF";    

    if ((($valueEnlightOverSellInt != '') or ($valueEnlightUnderSellInt  != '')) & (($valueEnlightOverSellInt & $valueEnlightUnderSellInt) != "0"))      {
    if (($finalPriceSellerInt >= $valueEnlightOverSellInt) or  ($finalPriceSellerInt <= $valueEnlightUnderSellInt)) {
    $fontColorSellInt = "#00FF00";
    } else if (($finalPriceSellerInt >= $valueEnlightOverSellInt) and  ($finalPriceSellerInt <= $valueEnlightUnderSellInt)) {
    $fontColorSellInt = "#FF00FF";
    } else {
    $fontColorSellInt = "#D00000";  
    }};

As you see I have a post form (not showed in code) that transmit to this file the values for: VAR underBuy < BUY PRICE < VAR overBuy; VAR underLocalSeller < LOCAL SELL PRICE < VAR overLocalSeller; VAR underIntSeller < INT SELL PRICE < VAR underIntSeller.

Firstly the code makes a check if the VAR received from form are EMPTY or = to 0; if it's not, the code collect all this data and make a check on $FINALPRICEBUYER (extracted from a file with json) for the 1st two VARs, than make a check on $FINALPRICESELLERLOCAL for the 3rd and the 4th VARs and so make a check on $FINALPRICESELLERINT for the 5th and the 6th VARs.

If $ FINALPRICEBUYER is between >= than 1st VAR or <= than 2nd VAR make font GREEN, else if $FINALPRICEBUYER between 3rd VAR and 4th VAR go VIOLET, else go RED.

The same for $FINALPRICESELLERLOCAL and $FINALPRICESELLERINT.

I did the first two conditions because user can insert one price limit or two.

Could you help me to understand what's I'm doing wrong? (Maybe it's only a matter of logic).

Here is the question:

This code doesn't work. Many time it return green font in spite of violet, so there should be something in the logic structure of math symbols (or PHP code of course) I can't get...

UPDATE 29.06.2015

Here is the code I'm using and adapting, starting from on of your examples.

// ENLIGHT NUMBERS ***************************************************

// get all input variables at once
$over_buy  = $_POST['enlightOverBuy'];
$under_buy = $_POST['enlightUnderBuy'];
$over_loc  = $_POST['enlightOverSloc'];
$under_loc = $_POST['enlightUnderSloc'];
$over_int  = $_POST['enlightOverSellInt'];
$under_int = $_POST['enlightUnderSellInt'];

$final_buy = $finalPriceBuyer;
$final_loc = $finalPriceSellerLocal;
$final_int = $finalPriceSellerInt;

// now set the colors
$buy = getFontColor( $over_buy, $under_buy, $final_buy );
$loc = getFontColor( $over_loc, $under_loc, $final_loc );
$int = getFontColor( $over_int, $under_int, $final_int );

So nothing different here. Now the function:

 // function to return color based on input
 function getFontColor( $over, $under, $final ) {

 // colors
 $white  = '#fff';
 $green  = '#0f0';
 $violet = '#f0f';
 $red    = '#d00000';

Fist step: if function get the "0" number font will be white so:

 if ($final != '0') {

Second step: if both the "value" are sent by the form

     if (!empty($over) and !empty($under)) {

Third step: if FINAL is a number between over and under

           if ($final >= $over && $final <= $under) {

go GREEN

                  return $green;

if not

           } else {

go RED

           return $red; 
           }

Fourth step: else if one of the "value", or the other one, is empty

 } else if (!empty($over) or !empty($under)) {

Fifth step: if FINAL is a number higher than "over" value, or lower than "under" value

        if ($final >= $over or $final <= $under) {

go GREEN

                return $green;

else go RED

        } else {
                return $red;
                }

Sixth step: in any other case, go WHITE

 } else {
        return $white;
 }
 }};

So I would have GREEN if $final is in between two values received from form, but if you have only one value input (so under or over), if $final is higher than over or lower than under, go GREEN. In any other caso, go RED, if $final is not "0".

Here is the full code without spaces:

// ENLIGHT NUMBERS ***************************************************

// get all input variables at once
$over_buy  = $_POST['enlightOverBuy'];
$under_buy = $_POST['enlightUnderBuy'];
$over_loc  = $_POST['enlightOverSloc'];
$under_loc = $_POST['enlightUnderSloc'];
$over_int  = $_POST['enlightOverSellInt'];
$under_int = $_POST['enlightUnderSellInt'];

$final_buy = $finalPriceBuyer;
$final_loc = $finalPriceSellerLocal;
$final_int = $finalPriceSellerInt;

// now set the colors
$buy = getFontColor( $over_buy, $under_buy, $final_buy );
$loc = getFontColor( $over_loc, $under_loc, $final_loc );
$int = getFontColor( $over_int, $under_int, $final_int );

 // function to return color based on input
 function getFontColor( $over, $under, $final ) {

 // colors
 $white  = '#fff';
 $green  = '#0f0';
 $violet = '#f0f';
 $red    = '#d00000';

 if ($final != '0') {
     if (!empty($over) and !empty($under)) {
           if ($final >= $over && $final <= $under) {
                  return $green;
           } else {
           return $red; 
           }
 } else if (!empty($over) or !empty($under)) {
        if ($final >= $over or $final <= $under) {
                return $green;
        } else {
                return $red;
                }
 } else {
        return $white;
 }
 }};

It seems correct to me, but I'm receiving a strange result, the echos you will see were created by me to make a check of function behavior.

Please do not consider my Italian comments, but the result is a non-sense, considering in:

• 1st example: GREEN (0.127839 is NOT under 0.125, so should be RED) RED (correct) - GREEN (correct)
• 2nd example: GREEN (0.154761 is NOT under 0.125, so should be RED) - RED (correct) - GREEN (correct)
• 3rd example: GREEN (0.14442 is NOT under 0.125, so should be RED) - RED (correct) - GREEN (correct)
• 4th example: GREEN (0.129195 is NOT under 0.125, so should be RED) - RED (correct) - RED (correct).

How is it possible that using the same function you get always an error on 1st usage and correct answer on the next 2 uses?

FINAL UPDATE

Here is the code:

 $over_buy  = number_format($_POST['enlightOverBuy'], 6);
 $under_buy = number_format($_POST['enlightUnderBuy'], 6);
 $over_loc  = number_format($_POST['enlightOverSloc'], 6);
 $under_loc = number_format($_POST['enlightUnderSloc'], 6);
 $over_int  = number_format($_POST['enlightOverSellInt'], 6);
 $under_int = number_format($_POST['enlightUnderSellInt'], 6);


 $final_buy = $finalPriceBuyer;
 $final_loc = $finalPriceSellerLocal;
 $final_int = $finalPriceSellerInt;

 // now set the colors
 $buy = getFontColor( $over_buy, $under_buy, $final_buy );
 $loc = getFontColor( $over_loc, $under_loc, $final_loc );
 $int = getFontColor( $over_int, $under_int, $final_int );


 // function to return color based on input
 function getFontColor( $over, $under, $final ) {

 // colors
 $white  = '#fff';
 $green  = '#0f0';
 $red    = '#FF3300';

 if ($final != '0') {
    if (!empty($over) and !empty($under)) {
        if ($final >= $over && $final <= $under) {
                return $green;
        } else {
                return $red; 
                }
 } elseif (!empty($over)) {
                return ($final >= $over) ? $green : $red;
 } elseif (!empty($under)) {
                return ($final <= $under) ? $green : $red;
 } else {
    return $white;
    }
 }};

Answer 1

You must check the most restrictive condition first. See this code:

// case A
if($z>$x && $z<$y) {}
// case B
if($z>$x || $z<$y) {}

Outcome:

$x | $y | $z | case A | case B
10   20    5    false     true
10   20   15     true     true
10   20   25    false     true

Whenever case A evaluates as true, case B also does. But the reverse situation might not. Therefore if you put case B first in an IF ELSE, it will always return true before reaching case A.
Fixed code sample:

$valueEnlightOverBuy  = $_POST["enlightOverBuy"];
$valueEnlightUnderBuy = $_POST["enlightUnderBuy"];

// GREEN PRICE BUY
$fontColorBuy = "#FFF";

// user has provided price boundaries
if($valueEnlightOverBuy !== '' && $valueEnlightUnderSell !== '') {

   // satisfies both boundaries (violet, should be green ?)
   if($finalPriceBuyer >= $valueEnlightOverBuy && $finalPriceBuyer <= $valueEnlightUnderBuy) {
      $fontColorBuy = "#FF00FF";
   }
   // satisfies at least one boundary (green, should be violet ?)
   elseif($finalPriceBuyer >= $valueEnlightOverBuy || $finalPriceBuyer <= $valueEnlightUnderBuy) {
      $fontColorBuy = "#00FF00";
   }
   // outside the boundaries (wrong boundaries order) (red)
   else {
      $fontColorBuy = "#D00000";
   }
}

Instinctively, I get the feeling that you actually want the AND condition to be green and the OR condition to be violet, but I can be wrong about that. And I must point out the red condtion only fires if the boundaries are set in the wrong order, although that seems appropriate.

Regarding update 2015-06-29:

In this code:

elseif(!empty($over) or !empty($under)) {
   if ($final >= $over or $final <= $under) {
      return $green;
   } 
   else {
      return $red; 
   }
}

In your examples, you get $green because $over is evaluated as zero, therefore any positive value will return green. You must only evaluate the value that is not empty.

elseif(!empty($over)) {
   return ($final >= $over) ? $green : $red;
}
elseif(!empty($under)) {
   return ($final <= $under) ? $green : $red;
}

Answer 2

Many things to address here...

It looks like you could use a good reading on:

• truth tables and karnaugh maps to help you get the apropriate expresion and simplify it
• is_numeric, to check for a numeric input
• operators in general, and in particular operator precedence
• The diference between & (bitwise operator) and && (logical operator)
• How to properly use if...elseif expresions. The elseif condition only gets evaluated when the if condition is not met, so if you put something like this in your code where the elseif condition is a subset of the if condition, the elseif condition will only be evaluated when it is not met, rendering it useless because the code inside it will never be executed:

.

if (($finalPriceBuyer >= $valueEnlightOverBuy) or ($finalPriceBuyer <= $valueEnlightUnderBuy)) {
    $fontColorBuy = "#00FF00";
} else if (($finalPriceBuyer >= $valueEnlightOverBuy and $finalPriceBuyer <= $valueEnlightUnderBuy)) { ... }

More importantly, be lazy and DRY. If you are doing the same thing twice, put it into a function:

function getColor($value, $limitOver, $limitUnder, $colors){
    $underCheck = !is_numeric($limitUnder) || $value >= $limitUnder;
    $overCheck = !is_numeric($limitOver) || $value <= $limitOver;
    if ($underCheck && $overCheck) {
        // all valid bounds are satisfied
        return $colors['none'];
    } else if ($overCheck){
        // valid lower bound exists and is not satisfied 
        // and upper bound does not exist or is not valid or is satisfied
        return $colors['under'];
    } else if ($underCheck){
        // valid upper bound exists and is not satisfied 
        // and lower bound does not exist or is not valid or is satisfied
        return $colors['over'];
    } else {
        // both bounds exist and none of them are satisfied
        return $colors['both'];
    }
}

$colors = array (
    'both' => "#FF00FF",
    'over' => "#D00000",
    'under' => "#FFF",
    'none' => "#00FF00",
);

$colorBuy = getColor(
    $finalPriceBuyer,
    $_POST["enlightOverBuy"],
    $_POST["enlightUnderBuy"],
    $colors 
);

$colorLocal = getColor(
    $finalPriceSellerLocal,
    $_POST["enlightOverSloc"],
    $_POST["enlightUnderSloc"],
    $colors 
);

$colorInt = getColor(
    $finalPriceSellerInt,
    $_POST["enlightOverSellInt"];,
    $_POST["enlightUnderSellInt"],
    $colors 
);

You have not been exactly clear on your definition of the problem, so it is up to you to check if I have correctly interpreted your intentions and make the required modifications to suit your exact needs, but you get the idea.

Answer 3

NOTE: This question should likely be moved to the code review site.

I think your main problem is coding style and organization. Your code is hard to read and therefore harder to debug. Particularly when you are trying to implement complex logic, making sure that you have clean code is helpful. Here are some suggestions.

1. Use shorter variable names
2. User proper indentation
3. Use built-in PHP functions to simplify, e.g. empty()
4. Use functions to get rid of repetition
5. Group similar tasks in one place, e.g. getting values of $_POST variables

So first, I'll rewrite your code, following these suggestions:

// function to return color based on input
function getFontColor( $over, $under, $final ) {
    // colors
    $white  = '#fff';
    $green  = '#0f0';
    $violet = '#f0f';
    $red    = '#d00000';

    if ( !empty($over) and !empty($under) ) {
        if ( $final >= $over || $final <= $under ) return $green;
        if ( $final >= $over && $final <= $under ) return $violet;
        return $red;
    } else {
        return $white;
    }
}

// get all input variables at once
$over_buy  = $_POST['enlightOverBuy'];
$under_buy = $_POST['enlightUnderBuy'];
$over_loc  = $_POST['enlightOverSloc'];
$under_loc = $_POST['enlightUnderSloc'];
$over_int  = $_POST['enlightOverSellInt'];
$under_int = $_POST['enlightUnderSellInt'];

// now set the colors
$buy = getFontColor( $over_buy, $under_buy, $final_buy );
$loc = getFontColor( $over_loc, $under_loc, $final_loc );
$int = getFontColor( $over_int, $under_int, $final_int );

Once, we've done that, we can see at least one problem. The getFontColor() function above will NEVER return $violet because you have the OR statement before the AND statement. To correct that, the function should be rewritten as:

// function to return color based on input
function getFontColor( $over, $under, $final ) {
    // colors
    $white  = '#fff';
    $green  = '#0f0';
    $violet = '#f0f';
    $red    = '#d00000';

    if ( !empty($over) and !empty($under) ) {
        if ( $final >= $over && $final <= $under ) return $violet;
        if ( $final >= $over || $final <= $under ) return $green;
        return $red;
    } else {
        return $white;
    }
}

However, even with this revision, I'm still not sure the code does what you expect it to do. In any case it should be easier to debug, now that it is simpler and cleaner. Hope this helps!