2
\$\begingroup\$

first time visiting Code Review. I figured this was a more appropriate spot than SO for this question. Here goes...

Currently, I have a php function that converts bbcode format text in to HTML (among other things). A code snippet is as follows...

function bbcode_format ($str) {  

 $simple_search = array( 
    '/\[b\](.*?)\[\/b\]/is',  // bold
    '/\[i\](.*?)\[\/i\]/is',  // italics
    '/\[u\](.*?)\[\/u\]/is',  // underline
)

  $simple_replace = array(  
    '<strong>$1</strong>',  
    '<em>$1</em>',  
    '<u>$1</u>'
)

    $str = preg_replace ($simple_search, $simple_replace, $str);  
    return $str;
}

The code itself works fine. However, in my actual code, there are currently around 60 items in each array I'm checking for, as I also used it to convert smileys, for example.

My concern is that this approach doesn't seem to be scalable, at least in the sense that the array just keeps growing and growing.

I'm wondering if anyone has another approach as to how this might be done, or any suggestions to improve my current method. Thanks!

Additional Info

Thanks for the suggestions so far... I may not have used the best examples above, as b / i / u are only a small set of what I'm replacing. I probably have more "smiley" replacements, than anything, so those generally look like this...

'/:\)/is',          // smiley :)
'/:d/is',           // laugh smiley :D
'/\:gift1\:/is',    // Holiday Gift 1 :gift1:
'/\:gift2\:/is',    // Holiday Gift 2 :gift2:

Each of those is then replaced with an tag.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

You have two issues: how to represent all of the desired transformations as code, and how to perform the substitutions efficiently.

To address the data representation issue — you want to use an associative array.

$simple_replacement_patterns = array(
    '/\[b\](.*?)\[\/b\]/is' => '<strong>$1</strong>',
    '/\[i\](.*?)\[\/i\]/is' => '<em>$1</em>',
    '/\[u\](.*?)\[\/u\]/is' => '<u>$1</u>',
);

However, there is a lot of redundancy there, which violates the Don't Repeat Yourself principle. Ideally, your source code should contain just

$simple_tag_translations = array(
    'b' => 'strong',
    'i' => 'em',
    'u' => 'u',
);

… and the computer build $simple_replacement_patterns for you.

Consider mapping [b]bold text[/b] to <b>bold text</b>, [i]italic text[/i] to <i>italic text</i>, and [u]underlined text[/u] to <u>underlined text</u>. Not only would the mapping be trivial (BBCode is inspired by HTML designed to be easily translated, after all), you would also not be inferring semantic value into the style. For example, maybe the BBCode text was italicized because it is a title in a bibliography entry, not because it is to be emphasized.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the info here... I like the use of the associative array. This definitely keeps readability up... as most of the comments in my arrays are there so that I can keep track of which line is which between them. Using this would help a lot. \$\endgroup\$
    – Charlie74
    Dec 8, 2013 at 20:25
  • \$\begingroup\$ Also, until you mentioned it, I always thought the use of <i> and <b> were deprecated, or at least frowned upon. But I read up on it on MDN a bit, and your suggestions are definitely valid. \$\endgroup\$
    – Charlie74
    Dec 8, 2013 at 20:26
2
\$\begingroup\$

I suspect that you might get better performance by performing one substitution rather than one per tag. You'll have to use PCRE back references and preg_replace_callback().

$simple_tag_translations = array(
    'b' => 'strong',
    'i' => 'em',
    'u' => 'u',
);

# $bb_pattern = '#\[(b|i|u)\](.*?)\[/\1\]#is';
$bb_pattern = '#\[(' .
                  implode('|', array_keys($simple_tag_translations)) .
              ')\](.*?)\[/\1\]#is';

return preg_replace_callback(
    $bb_pattern,
    function($matches) {
        $bb_tag = $matches[1];
        $body = $matches[2];
        $html_tag = $simple_tag_translations[$bb_tag];
        return "<$html_tag>$body</$html_tag>";
    },
    $str
);

The usual / is a poor choice for your PCRE delimiter since you are trying to match closing tags. You might as well pick another character to help avoid Leaning Toothpick Syndrome.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.