2

I have a JS function that takes JSON data and creates an HTML table. As my project developed, I needed one specific table to be created with a button - so I added an option to the function and an appropriate if statement. Then I decided I wanted to add a table colour option - another option; another if statement.

Finally, I now wish to add another option to colour rows depending on the content of the a parameter in each row of data. But that is ONLY for a specific set of JSON data that I will send - so I now need to add another option and a very specific if statement to catch this table type.

What I'd like to know: is there a better way of doing this? Most of the code to create the table is generic so don't wish to create new functions for each use case. But the function is now becoming full of options and ifs and not sure if this considered bad practice.

Edit: after some further searching, I think I am asking about overloading best practices. But, from what I understand, overloading in JS is about having lots of ifs or switch...case which is essentially I am not sure about the best way to do or if it is even appropriate.

Some code with questions in the comments (edited to keep it brief):

    function CreateTableFromJSON(jData, colour, addBtnToCol) {

table.className = "ui collapsing striped table unstackable " + colour; // Adds the colour option.


// Add JSON data to the table as rows.
for (var i = 0; i < jData.length; i++) {
    var tr = tbody.insertRow(-1);

    for (var j = 0; j < col.length; j++) {
        var tabCell = tr.insertCell(-1);

// This 'option' adds a button if the column is a particular value
// Is better practice to keep the main function (create table) generic and then call another (sub?) function to add the button to the created table?

        if(col[j] === addBtnToCol){
            var btnAdd = document.createElement('button');
            btnAdd.type = "button";
            tabCell.appendChild(btnAdd);
        }
        else
            tabCell.innerHTML = jData[i][col[j]];
    }
}
}
  • 2
    @gnat how does that releate to the question? Perhaps others would understand better, why you think this might be a duplicate, if you add a (short) reason. – Thomas Junk Apr 25 '18 at 7:53
  • 1
    To op: note that despite gnat's "possible duplicate" (is gnat a bot?), this question isn't at all specific to OOP, but rather a fundamental code construction question applicable to (almost) every paradigm of programming. – jleach Apr 25 '18 at 9:52
4

Adding on to Thomas Junk's answer: once your parameters are a handled a bit better, you can split up your functions something like this:

function makeMyTableThing(data, options) {

    var workup = generateBaseTable(data);

    if (options.option1 === 1) {
        workup = addOption1(workup, data);
    }

    if (options.option2 === 2) {
        workup = addOption2(workup, data);
    }

    return workup;  
}

function generateBaseTable(data) {
    return stuff;
}

function addOption1(workup, data) {
    return stuff;
}

function addOption2(workup, data) {
    return stuff;
}

This gives you one "entry function" whose single responsibility is to choreograph the rest of the functions depending on the request (obviously your workflow may/will look much different, but for general demonstration purposes...)

Taking it a bit further, one can wrap this up into a something that hides the implementation by using an object that returns the main entry function, or classes, or a module, or any number of other methods.

2

From what you write, your code looked initially like

function(data, withButton){ 
    //logic
}

And over time it got something like

function(data, withButton, option2, option3 /*and so on*/){ 
    //logic
}

You could improve, if you refactor your code to take only two parameters:

function(data, options){ 
    //logic
}

And with ES6 you have the nice desctructuring assignment, which helps you extract the options

{option1, option2, option3} = {option1: "1",option2: "2", option3: "3"}

That should improve your code.


Besides, if you notice, that your function becomes hard to maintain, why not split it up into several functions, which only cover a part of what you want to do?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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