0
\$\begingroup\$

An API provides data as some sort of data table, column and rows.

I have written a function to get JS Objects from this DataTable.

function processAggregateData(data) {
    dataRows = data.data[0].rows
    var returnArray = [];
    for (var i = 0; i < dataRows.length; ++i){
        returnArray.push({
            index: dataRows[i].fld[0].v,
            average: dataRows[i].fld[1].v,
            min: dataRows[i].fld[2].v,
            max: dataRows[i].fld[3].v,
            sum: dataRows[i].fld[4].v,
            target: dataRows[i].fld[5].v,
            LLL: dataRows[i].fld[6].v,
            LL: dataRows[i].fld[7].v,
            L: dataRows[i].fld[8].v,
            H: dataRows[i].fld[9].v,
            HH: dataRows[i].fld[10].v,
            HHH: dataRows[i].fld[11].v,
            status: dataRows[i].fld[12].v
        })
    }
    return returnArray;
}

I would like to improve it, in particular:

  • There is a lot of 'magic constants', mapping a column number to an object property. I have tried to extract them, but have not found a way to do it without doubling the size of the function. Is that suitable here?

  • Of course, any improvement of the code is most welcome.

A note, to end, there is users on IE, so ECMA6 is not possible.

Thank you for your review :)

\$\endgroup\$
  • \$\begingroup\$ I like the explicit mapping of labels to column indices. I would probably rename returnArray to result or columns, declare var dataRows and var dataRow = dataRows[i]; inside the loop and keep it as is. \$\endgroup\$ – le_m Dec 8 '17 at 17:05
1
\$\begingroup\$

There isn't really much you can do unless you know the objects property name. I refactored the properties into an array, so you just can iterate through them as they are reused so many times.

function processAggregateData(data) {
    var dataProperties = ['index', 'average', 'min', 
                          'max', 'sum', 'target', 
                          'LLL', 'LL', 'L', 
                          'H', 'HH', 'HHH', 
                          'status'];
    dataRows = data.data[0].rows;
    var returnArray = [];
    for (var i = 0; i < dataRows.length; ++i){
        var dataObject = {};
        for(var j=0;j<dataPoperties.length;j++){
          dataObject[dataProperties[j]] = dataRows[i].fld[j].v;
        }
        returnArray.push(dataObject);
    }
    return returnArray;
}

As you can see there are no longer any magic constants.

NOTE: EcmaScript 6 is possible, you just need to transpile it into EcmaScript 5

\$\endgroup\$

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.