5
\$\begingroup\$

I am doing a lot of stuff with JavaScript and jQuery lately, but am not heading for clean code too much, as I am usually writing userscripts or small applications for myself.

Recently, a community I am part of announced that it will change their supported BB-Codes, as the tabmenu BB-Code had a possible safety issue. By now, hundreds of posts have been done with tabmenus and a lot of users were complaining that editing all these posts will take forever.

As I am a guy who likes to automate as much stuff as possible, I took the task and developed a small application that changes posts with the tabmenu BB-Code into a format that is supported by the forum.

Here are two examples of what the application does:

Original BB-Code:

[tabmenu]
[tab=Tab1]Tab1 Text
[tab=Tab2]Tab2 Text
[/tabmenu]

Note: tabmenu has a closing tag, tab does not and will contain all text until the next tab or until the closing [/tabmenu].

Result:

[h]Tab1[/h]Tab1 Text
[h]Tab2[/h]Tab2 Text

h turn into headlines, just as in HTML and force a linebreak.

As the tabmenu does not only support tabs, but also subtabs, here is the second example:

Original BB-Code:

[tabmenu]
[tab=Tab1]
[subtab=Subtab11]Subtab11 Text
[subtab=Subtab12]Subtab12 Text
[tab=Tab2]
[subtab=Subtab21]Subtab21 Text
[/tabmenu]

Note: If you have subtabs in you tabmenu, you can't write text into tab areas. Apart from that subtabs behave like tabs when getting their content.

Result:

[h]Tab1[/h]
[spoiler=Subtab11]Subtab11 Text
[/spoiler][spoiler=Subtab12]Subtab12 Text
[/spoiler][h]Tab2[/h]
[spoiler=Subtab21]Subtab21 Text
[/spoiler]

tabs are still being replaced with h. The behavior for subtabs is a bit different though. Here my application is taking the title from the subtab, turning it into a spoiler-Tag (not comparable with the spoiler-codes of Stack Exchange, the spoiler-BB-Code is more like an accordion) and placing the content of the subtab between the opening spoiler-Tag and the closing one.

$(function () {
    $('#buttonGenerate').on('click', function () {
        // The format of titles can be like this: [tab=Title], [tab="Title"] & [tab='Title']
        // the user can choose the format with this checkbox
        var bbFormat = $('input[name=bbCodeFormat]:checked').val();
        var titleTemplate;
        var regexTab;
        var regexSubTab;

        switch (bbFormat) {
            // Depending on which format was chosen, different regex patterns are used
            case '1':
            // Format: [tab=Title], allows certain special chars in the title
                regexTab = /\[tab=((\w|[ äöüÄÖÜ,.\-;:_#'+*!"§$%&/()=?ß^°])*)\]/g;
                regexSubTab = /\[subtab=((\w|[ äöüÄÖÜ,.\-;:_#'+*!"§$%&/()=?ß^°])*)\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g;
                titleTemplate = '###';
                break;
            // Format: [tab="Title"] allows any special chars in the title
            case '2':
                regexTab = /\[tab="((.| )*)"\]/g;
                regexSubTab = /\[subtab="((.| )*)"\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g;
                titleTemplate = '"###"';
                break;
            // Format: [tab='Title'] allows any special chars in the title
            case '3':
                regexTab = /\[tab='((.| )*)'\]/g;
                regexSubTab = /\[subtab='((.| )*)'\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g;
                titleTemplate = "'###'";
                break;
        }

        // Gather original BB-Code from textarea
        var input = $('#inputArea').val();

        // Replace the tabmenu BB-Code via Regex
        input = input.replace(regexSubTab, '[spoiler=' + titleTemplate.replace('###', '$1') + ']$3[/spoiler]');
        input = input.replace(regexTab, '[h]$1[/h]');
        input = input.replace('[tabmenu]', '').replace('[/tabmenu]', '');
        $('#outputArea').val(input.trim());
    });
});
\$\endgroup\$

3 Answers 3

2
\$\begingroup\$

Your code is good, but it could be improved:


Using an enum

If you use an enum, you can avoid Magic Numbers in your switch statement:

    switch (bbFormat) {
        // Depending on which format was chosen, different regex patterns are used
        case '1':

Whereas, doing:

var BB_FORMAT_TYPES = {
    NoQuotes: 1,
    DoubleQuotes: 2,
    SingleQuotes: 3
};

with:

switch (bbFormat) {
    case BB_FORMAT_TYPES.NoQuotes:
        break;
    case BB_FORMAT_TYPES.SingleQuotes:
        break;
    case BB_FORMAT_TYPES.DoubleQuotes:
        break;
}

However, using an enum is usually suited more for when the contents of each have different execution styles. In your case, it'd be better to move the code to an object, and have a single execution path referencing said object.

    var object = {
        1: {
            regexTab: /\[tab=((\w|[ äöüÄÖÜ,.\-;:_#'+*!"§$%&/()=?ß^°])*)\]/g,
            regexSubTab: /\[subtab=((\w|[ äöüÄÖÜ,.\-;:_#'+*!"§$%&/()=?ß^°])*)\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g,
            titleTemplate: '###'
        },
        2: {
            regexTab: /\[tab="((.| )*)"\]/g,
            regexSubTab: /\[subtab="((.| )*)"\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g,
            titleTemplate: '"###"'
        },
        3: {
            regexTab: /\[tab='((.| )*)'\]/g,
            regexSubTab: /\[subtab='((.| )*)'\]((.|\s)+?((?=\[subtab)|(?=\[\/tabmenu)|(?=\[tab)))/g,
            titleTemplate: "'###'"
        }
    };

Chaining:

Instead of having the replace on each line, try using chaining:

    input = input.replace(regexSubTab, '[spoiler=' + titleTemplate.replace('###', '$1') + ']$3[/spoiler]');
                 .replace(regexTab, '[h]$1[/h]');
                 .replace('[tabmenu]', '')
                 .replace('[/tabmenu]', '');

that way you can avoid looking up and writing to the input variable each time.

\$\endgroup\$
1
  • \$\begingroup\$ Exactly what I was looking for, especially the enums were something I was not aware of at all. \$\endgroup\$
    – Marv
    Jan 14, 2016 at 7:44
1
\$\begingroup\$

When you put regex in your code, I think you should describe what the regex does. So that if there's ever a problem in the script, you can understand what each regex is for.

You've done this already for the "tab" regex, but should do so as well for the subtabs.

\$\endgroup\$
2
  • \$\begingroup\$ Basically the comment applies on both, as only one format is allowed in the tabmenu at once. Good call on that though, I think I should explain what the regex does in general by breaking it up \$\endgroup\$
    – Marv
    Jan 13, 2016 at 10:10
  • \$\begingroup\$ @Pimgd do you have anything else to review about this code? As presented the answer is very minimal and perhaps should be a comment instead. \$\endgroup\$
    – Phrancis
    Jan 13, 2016 at 16:42
1
\$\begingroup\$

Two of your regexSubTab values are the same, you should set them as a constant at the top so that it's clear that they're the same. Names would also indicate what the regex value means/is used for. It might even be valuable to set all the regex strings as constants at the top, so it's easier to change them there, rather than having to dig through the body of the code.

It might be worth having a default case that explicitly logs an error, in case more options are possible later. Otherwise you'll just get errors from trying to call undetermined values.

\$\endgroup\$

Your Answer

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

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