patternjavascriptMinor
BBCode to HTML converter using functional programming
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:
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);
I'd like feedback on:
- structuring of the code. Should the functions
getHtmlTagandgetHtmlbe contained withinbbTagToHtmlTagsince they are coupled through therulesvariable?
- 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:
Could be
Similarly, a few more local variables wouldn't hurt. E.g.
Could be
-
More micro-level: The pattern in
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
There's a similar thing in your
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 `
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/[\\,!^${}[\]().*+?|<>&-]/gThe 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);/[\\,!^${}[\]().*+?|<>&-]/gContext
StackExchange Code Review Q#46751, answer score: 2
Revisions (0)
No revisions yet.