5

I have a PHP script that runs through an array of values through my own custom function that uses the PHP function preg_match. It's looking for a match with my regular expression being $valueA, and my string to search being $valueB, and if it finds a match, it returns it to $match, otherwise I don't want my IF statement to run.

Now, I'm having no problem with this IF statement running if the function finds a match (in other words, is TRUE);

if ($match = match_this($valueA, $valueB))
{
    //do this
}

HOWEVER, if I wanted to compare an additional condition to check if it is also TRUE, and only run the IF statement when both are TRUE, I run into problems;

if ($match = match_this($valueA, $valueB) && $x == 'x')
{
    //do this
}

Instead of the statement running as normal when both conditions are TRUE, I end up outputting a 1 from $match instead of what my $match value should be.

I can sort of see what's happening. My IF statement is just returning a literal boolean TRUE into $match, instead of my value returned from match_this();

Is this because I can only return one sense of TRUE?

I can get around the problem by having two nested IF statements, but for the sake of clean code I'd like to be able to compare multiple conditions in my IFs which includes functions that return values.

Are there different kinds of TRUE? If so, how do I compare them in this way? Or is there an easier method? I suppose I could just put a second IF statement inside my function and pass my second condition through the function, but then my function wouldn't be very clearly defined in terms of its purpose.

I've ran into this problem before and didn't quite know what I was looking for. Hope someone can help.

j08691
  • 204,283
  • 31
  • 260
  • 272
Martyn Shutt
  • 1,671
  • 18
  • 25
  • 1
    Operator precedence. Replace `&&` with `and` might be what you wanted to express. See http://en.wikipedia.org/wiki/Order_of_operations and http://php.net/manual/pl/language.operators.precedence.php – hakre Oct 26 '12 at 14:34
  • Yes, there's multiple kinds of expressions that can be evaluated to `true` in PHP. (1, -1, "hello", ...) See [the documentation](http://www.php.net/manual/en/language.types.boolean.php). – Maxime Morin Oct 26 '12 at 14:37
  • Useful documentation. Thank you. – Martyn Shutt Oct 26 '12 at 15:32

2 Answers2

5
if (($match = match_this($valueA, $valueB)) && $x == 'x') 
    ^                                     ^

You missed an extra set of brackets. Without the extra set of brackets around $match = match_this($valueA, $valueB), the evaluation of the && is performed first and finally result is assigned to $match.

Alternatively, you could use and (which has lower precedence than assignment =) instead of &&

PHP Operator Precedence

Anirudh Ramanathan
  • 46,179
  • 22
  • 132
  • 191
  • Ah, I understand. That's exactly what I needed to know. It seems very logical now that I see the solution. Thanks for the quick reply. – Martyn Shutt Oct 26 '12 at 15:02
-1
if ($match = match_this($valueA, $valueB))
           ^---wrong operator

You should be using == for equality tests. Right now you're just assigning the return values of match_this. Ditto for your other if()

as per the comments below, if you actually ARE wanting to do assignments within the if() structure, then you should slap a very-large-blinking-neon-bright comment/warning above the code to say that this is intended behavior. In most any language and most any other person looking at this code later on will assume it's a typo and change the assignment to an equality test.

Marc B
  • 356,200
  • 43
  • 426
  • 500
  • -1 He's not trying to check for equality. He *wants* to assign the return values... – Joseph Silber Oct 26 '12 at 14:35
  • Then what is the point of `&& $x = 'x'`? That boils down to $match being true regardless because the return value of an assignment is the value being assigned. so it's `$match = match_this(..) && true`. Assignments inside if() tests are invariably always typos – Marc B Oct 26 '12 at 14:37
  • 1
    What the OP is trying to do is equivalent to this: `$match = match_this(...); if($match && $x == 'x'){}` – Joseph Silber Oct 26 '12 at 14:40
  • Why would he say that then? "Instead of the statement running as normal when both conditions are TRUE, I end up outputting a 1 from $match instead of what my $match value should be." – Maxime Morin Oct 26 '12 at 14:42
  • Then he should definitely split the code up into an assignment and then a separate test. Look at the confusion it's causing now - what he's got now is going to bite him in the rump if anyone else comes across this code down the road. assignment-in-if is a BAD idea. – Marc B Oct 26 '12 at 14:43
  • @MarcB - I agree that **assignment-in-if is a BAD idea**. Put that in your answer instead. – Joseph Silber Oct 26 '12 at 14:46
  • @MaximeMorin - Since `$match` results in a boolean, the output of `$match` within the `if` block is `1` (or `true`), which is not what he's expecting... – Joseph Silber Oct 26 '12 at 14:48
  • @JosephSilber Thank you! After re-reading your previous comment, it makes sense now. It sure is confusing code! :) – Maxime Morin Oct 26 '12 at 15:05
  • I only wanted to run the IF statement if my function returned a match. Putting the function in an IF statement seemed the cleanest way to do it, but I can see it's caused some confusion! Cthulhu responded exactly with what I was trying to do in my code. – Martyn Shutt Oct 26 '12 at 16:00