Issue with False preg_match

Why does this code always go to the TRUE branch regardless of the value I put in for $body?


<?php
	$body = 'xxx';

	if (preg_match('~([0-9-]+)~', $body) !== false){
		echo 'Image Found';

	}else{
		echo 'Image Not Found';

	}
?>

According to the PHP Manual

preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or FALSE if an error occurred.
Warning

This function may return Boolean FALSE, but may also return a non-Boolean value which evaluates to FALSE. Please read the section on Booleans for more information. Use the === operator for testing the return value of this function.

So ‘xxx’ should return ‘0’ and since I am using ‘!==’ shouldn’t things resolve to the ELSE branch?? :-/

Thanks,

Debbie

No, as FALSE in this scenario means your regex is invalid. You want to check against 0 for no match. So if you use !== 0, you will get the expected result.

Yeah, you have to watch out when using the strict comparisons.


1 == true // true
1 === true // false
0 == false // true
0 === false // false

cpradio,

Glad to catch you online, because this problem came about when I was modifying some code that you gave me that I apparently didn’t understand.

Rewinding…

You helped me with this code…


	// Find all Cross-Links in Article.
	if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER) !== false){
		// Loop through array.
		foreach ($matches as $match){
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}

Preferring a “balanced” IF-THEN-ELSE, I made this tweak…


	// Find all Cross-Links in Article.
	if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER) !== false){
		// Cross-Links Found.

		// Loop through array.
		foreach ($matches as $match){
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}else{
		// Cross-Links Not Found.
		// Do nothing...
	}

But apparently this code doesn’t do what I thought or intended, right?

So, some questions…

1.) I suppose having the !== false is good because it catches when my preg_match_all fails, right??

2.) If I want to keep the preg_match_all, then how do I make my IF-THEN-ELSE still work?

Or is that not needed?

Thanks,

Debbie

Yes, granted it should never fail since it isn’t generated by code and is hard coded, but nonetheless, I usually keep an if statement around it (in case someone changes the regex for example).

That entirely depends on “what will you do if there are no matches?” If you plan to do nothing, then you don’t need an else, as it won’t provide anything useful to you or your users (whereas, the if/else for the preg_match_all is necessary so you know the regular expression failed entirely and it didn’t search for any cross-links.

If you desperately wanted an else, you would put it inside your if block and check the sizeof($matches) != 0, then run your foreach, else do something else.

Yeah, it’s not really working the way you think it is working. preg_match_all !== false doesn’t check to see if there are matches. It just makes sure there wasn’t an error when calling the function. The function creates $matches = array(); to store any results. If there are no results, $matches will be an empty array. I think you’re trying to do something like this:


    // Find all Cross-Links in Article. 
    if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER) !== false){ 
        if (count($matches)) { // Matches found
            // Loop through array. 
            foreach ($matches as $match){ 
                $body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body); 
            }
        } else { // No matches found
            // do something
        }

    } 


It was working exactly the way I intended it :wink: But I didn’t explain it well it enough in my prior thread, the great thing is the foreach will just exit because $matches would be an empty array on its own, which is why you don’t need the inner IF statement UNLESS you want to do something if there are 0 matches found. Of course, you were probably writing up your response as I was writing mine :wink:

Lol I was talking to her. I guess you responded before I finished typing.

:slight_smile: I would say “nana boo boo, I beat you” but that just seems too immature :stuck_out_tongue: Good to see you around @kduv ;

I usually check out the forums every day or so, but lately you and cups have been getting to the posts before me and I haven’t had much of anything to add. I never really feel right posting a “yeah me too” reply. It just seems like a waste.

But yeah, I do agree with you. Unless she wants to do something when no matches are found, I’d leave it like it was as the foreach will take care of it.

If the syntax of my Regex was invalid, since it is also hard-coded, wouldn’t I get a compile error or a runtime error so I’d know my Regex is syntactically invalid?

On the code you helped me out with last week, I had this…


	// Find all Cross-Links in Article.
	if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER) !== false){
		// Cross-Links Found.

		// Loop through array.
		foreach ($matches as $match){
			// Pass sub-array to function which creates a new Article URL,
			// and then replaces each Cross-Link "placeholder" with the new link.
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}else{
		// Cross-Links Not Found.
		// Do nothing...
	}

I guess that is wrong, right?

I tried re-writing things to look like this, but I’m still not sure if it is necessary…


	if (($result = preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER)) === FALSE){
		// Bad Pattern.
			// Now what do i do???
					
	}else if ($result){
		// Cross-Links Found.

		// Loop through array.
		foreach ($matches as $match){
			// Pass sub-array to function which creates a new Article URL,
			// and then replaces each Cross-Link "placeholder" with the new link.
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}else{
		// Cross-Links Not Found.
		// Do nothing...
	}

Does that look better?

Off Topic:

Sorry, I haven’t eaten since yesterday, and my brain is fading quickly before supper, but since you are here, I didn’t want to miss your help!!

Before this whole topic came up, I would have normally just coded things this way…


	// Find all Cross-Links in Article.
	if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER)){
		// Cross-Links Found.

		// Loop through array.
		foreach ($matches as $match){
			// Pass sub-array to function which creates a new Article URL,
			// and then replaces each Cross-Link "placeholder" with the new link.
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}else{
		// Cross-Links Not Found.
		// Do nothing...
	}

Isn’t this last way good enough??

Thanks,

Debbie

I wasn’t thinking that at all, but you bring up another interesting point!!

Hmm… :scratch:

Debbie

I don’t think it will return false for an invalid regex syntax. That will most likely just return no results. When it says “returns false on error” it means something like setting the offset higher than the subject length.

cpradio (and kduv), I am ready to PASS OUT from starvation and working 10 hours straight…

If you get a chance later tonight, please let me know what you think about this related thread…

How to display Images for an Article

Off to supper for now…

Thanks, and I’ll respond when I can!!!

Debbie

I don’t think it would because at the JIT time, it is just a string, it isn’t until preg_match_all tries to use it, that is validates it and executes the regular expression. Since the method only returns true or false, my guess is you may get an error/notice/warning, but if you have those disabled in Production from being displayed (like you should), the true/false is all you can depend on.

Yes, that is acceptable, EXCEPT your comments in the ELSE are incorrect, it doesn’t mean Cross-Links Not Found, it means, the match statement failed because someone changed the regex so that it wasn’t valid syntax (such as using (.?) instead of (.?) (my dyslexia always causes me to type that one wrong; and I do realize your regular expression doesn’t currently contain those characters).

As for this one

The ELSE will never run, as your first condition already handles a FALSE $result, remember 0 matches is still a TRUE result from preg_match_all, so you would have to check within your ELSE IF that count($matches) or sizeof($matches) is NOT 0 to detect NO MATCHES.

Again, I ONLY recommend doing that ELSE, if you plan to do something about NO MATCHES being found. If you don’t plan to do anything, leave it as


	// Find all Cross-Links in Article.
	if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER)){
		// Cross-Links Found.

		// Loop through array.
		foreach ($matches as $match){
			// Pass sub-array to function which creates a new Article URL,
			// and then replaces each Cross-Link "placeholder" with the new link.
			$body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
		}

	}else{
		// preg_match_all failed to run
		// Do nothing...
	}

[ot]

Actually, I’ve ran into several threads where you beat me :slight_smile: And must say, I really do appreciate your responses.[/ot]

Actually, this function WILL return the number of results, or boolean false on error.

So this will work:


    // Find all Cross-Links in Article.
    if (preg_match_all('/\\\\{url=([a-zA-Z0-9-]+)\\\\}/', $body, $matches, PREG_SET_ORDER)){
        // Cross-Links Found.

        // Loop through array.
        foreach ($matches as $match){
            // Pass sub-array to function which creates a new Article URL,
            // and then replaces each Cross-Link "placeholder" with the new link.
            $body = str_replace($match[0], generateArticleCrossLink($dbc, $match), $body);
        }

    }else{
        // Cross-Links Not Found.
        // Do nothing...
    }  

That very well could be, the manual is vague on that aspect, and I guess we could denote that as being the case due to version 5.3.6 providing “Returns FALSE if offset is higher than subject length.”, but I digress there are likely other scenarios (I hope) that would return false (but maybe there aren’t).

Apart from the comment in the ELSE still being wrong, I agree. As the else does not mean Cross-Links Not Found, it means, preg_match_all did not run because it had an error.

It would still fire the ELSE if no matches are found as the function would return a 0 instead of a boolean false (or positive int in the case of one or more matches). The only downside would be that the ELSE would fire if there are no matches and if there is an error since you’re only testing for true/false and not boolean true/false.

She may or may not be interested in differentiating between the two, but I personally like to make sure errors are handled correctly, and I of course differentiate between an error and an empty result set.

Bah, I mis-read the manual. I thought it only returned TRUE or FALSE, my bad. You are correct.