HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

BBCode to HTML converter using functional programming

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bbcodeprogrammingusingfunctionalhtmlconverter

Problem

I was inspired to write a BBCode to HTML converter as I'm currently learning functional programming. I wanted achieve functional cohesion. jsFiddle

I'd like feedback on:

  • structuring of the code. Should the functions getHtmlTag and getHtml be contained within bbTagToHtmlTag since they are coupled through the rules variable?



  • any regex expressions that can reduce the code base



  • any changes to the data model to reduce complexity



Below is the newer version of the code. This version is slightly less coupled compared to the original version and the functions are separated to allow for more reuse. This new version doesn't involve any recursion, so it may be more efficient.

I suspect the cost of reuse is an increase in code base. Although, I'm sure there's more regex magic that can be used to slim down the code base.

```
/ Escapes a string for use with regular expressions /
function escapeString(input) {
return input.replace(/([\,\!\\\^\$\{\}\[\]\(\)\.\*\+\?\|\\-\&])/g,
function(c){return "\\" + c;});
}

/ replaces all /
function replaceAll(search, input, replacement, ignore) {
return input.replace(
new RegExp(escapeString(search), "g"+(ignore?"i":"")),
replacement);
};

/ Check if an index is event /
function isIndexEven(val, index, context) {
return (index % 2 === 0);
};

/ Sets the attributes on an HTML string given a set of attributes /
function setHtmlAttrs(htmlCode, attrs) {
return attrs.reduce(function(htmlCode, attr, index) {
var re = new RegExp('\\$' + (index + 1), 'g');
return htmlCode.replace(re, attr);
}, htmlCode);
};

/ Gets the html tag(s) for a bbCode tag /
function eleFragsToHtml(isClosing, elementFragments) {
return '';
};

/ Converts a single bbCode Tag to Html /
function bbTagToHtmlTag(rules, tag) {
var attrs = tag.replace(/\[\w+|\]/g, '').trim().split(' ');
var key = tag.replace(/\[\/?| .+|\]/g, '');
var isClosing = /\//.test(tag);

Solution

-
Overall, it's interesting, but quite hard to read. The indentation is kinda funky, and it would be easier to parse with all those ternary branches. For instance, this:

var htmlTemplate = (isClosing ? rules[key].map(function(rule) { 
                                    return rule.filter(isIndexEven)
                            }).reverse()
                    : rules[key]);


Could be

var htmlTemplate = rules[key];
if (isClosing) {
  htmlTemplate = htmlTemplate.map(function (rule) { 
    return rule.filter(isIndexEven);
  }).reverse();
}


Similarly, a few more local variables wouldn't hurt. E.g.

return input.replace(new RegExp(escapeString(search), "g"+(ignore?"i":"")), replacement);


Could be

var flags = "g" + (ignore ? "i" : ""),
    regex = new RegExp(escapeString(search), flags);
return input.replace(regex, replacement);


-
More micro-level: The pattern in escapeString can be simplified a great deal; you don't have to escape everything inside a character class, nor do you need the capture group. I get something like this

/[\\,!^${}[\]().*+?|<>&-]/g


The only things that need escaping is the backslash, the close square bracket, and the dash - unless you just put the dash at the end (if it's in the middle somewhere, it'll be interpreted as a range like 0-9).

There's a similar thing in your rules; you don't need to escape double quotes inside single-quoted strings.

I should add, though, that I haven't had the time to do a high-level review of all the regexp magic; I've looked a the individual patterns, not whether some can be combined, elided, or otherwise refactored.

-
Speaking of the rules: Whenever possible use class names or simply rely on HTML tag names instead of hardcoded style attributes. For instance, the ` tag exists in HTML, just like , so you don't need a styled span element. And the img tag doesn't need a style attribute; just declare the style for img tags in a CSS file. The point is, as with all things CSS, that you won't have to change your code (behavior) to change the look (presentation).

-
Some function names are pretty terse. E.g.
eleFragsToHtml is a bit too truncated, if you ask me (especially considering the 2nd argument is spelled out as elementFragments)

-
I'd define the unchanging regex patterns outside the functions they're in now, since they're static - or define some more functions, to make it more descriptive. For instance, this

var attrs = tag.replace(/\[\w+|\]/g, '').trim().split(' ');
var key = tag.replace(/\[\/?| .+|\]/g, '');
var isClosing = /\//.test(tag);


Could be

var attrs     = tagAttributes(tag),
    key       = tagName(tag),
    isClosing = tagIsClosing(tag);


(yes, I've combined the
var statements).

The
tag* prefix on all those functions is of course a hint that they might benefit from being namespaced in an object. Or just have one parseTag function that returns an object with name, attributes, and closing properties.

-
Lastly, it'd be nice to stick all of this in an IIFE, like

var bbCodeToHtml = (function () {
  // shared variables, regex patterns here

  // various helper functions

  return function (string) {
    // the function that ties it all together
  }
}());


The idea is that the global namespace isn't polluted with a lot of "niche" functions; all you'll see is a function called
bbCodeToHtml.

FYI, it's a toss-up whether the function should be
bbCodeToHTML or stay as it is. The DOM is inconsistent in this regard (you have the innerHTML and namespaceURI properties, but you also have functions like getElementById` - and I doubt they mean the Freudian "id" but rather "ID".

Code Snippets

var htmlTemplate = (isClosing ? rules[key].map(function(rule) { 
                                    return rule.filter(isIndexEven)
                            }).reverse()
                    : rules[key]);
var htmlTemplate = rules[key];
if (isClosing) {
  htmlTemplate = htmlTemplate.map(function (rule) { 
    return rule.filter(isIndexEven);
  }).reverse();
}
return input.replace(new RegExp(escapeString(search), "g"+(ignore?"i":"")), replacement);
var flags = "g" + (ignore ? "i" : ""),
    regex = new RegExp(escapeString(search), flags);
return input.replace(regex, replacement);
/[\\,!^${}[\]().*+?|<>&-]/g

Context

StackExchange Code Review Q#46751, answer score: 2

Revisions (0)

No revisions yet.