
Originally Posted by
cpradio
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.
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...
PHP Code:
// 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...
PHP Code:
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...
PHP Code:
// 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
Bookmarks