0
\$\begingroup\$

I'm flattening a JSON structure where objects have their interaction type appended at the end, for use in an HTML/PDF generated file. I have variable names for Checkboxes, Text inputs, TextCheck is a combined checkbox/input that needs to be broken apart for my logic. Lastly, I have a Radio Index and Radio Label ojbect in which Radio Label is an array of options to choose from, and Radio Index is the index which is filled, or null for unfilled. However, I have variables in the HTML file which need an associated JSON key so I must generate these.

Kind of new to the professional world and javascript so I'd like to see where I can beautify code, work out poor blocks, replace unsound variable names, etc. My coding style given is very relaxed, but mostly avoiding Else If, instead utilizing early exit, as well as having bracket openings on the same line.

JSON Snippet:

"billingSelfPayObject":                 
{      
    "Amount_Text":null,                                     
    "Credit Card_RadioIndex":null,                           
    "Credit Card_RadioLabels":["MasterCard", "Visa", "Discover", "American Express"],
},  
"patientQuestionnaire":
{
    "generalQuestions":
    {
        "indication":                                 
        {
            "Diagnostic_Checkbox":null,
            "Family history_Checkbox":null,
            "Known variant_Checkbox":null,
            "Presymptomatic_Checkbox":null,
            "Other (Specify)_TextCheck":null
        },
        "ICD-10 Code(s)_Text":null,                     
        "Clinical diagnosis_Text":null                 
    }, ...

Code:

function isDigit(num)
{
    if(typeof(num)==='number' || typeof(num)==='string')
    {
        return !isNaN(parseInt(num, 10));
    }
    return false;
}

function mergeObjects(obj1, obj2)
{
    for(var key in obj2)
    {
        if(!(key in obj1))
        {
            obj1[key] = obj2[key];
        }
    }   
}

function processKVFromObject(value, currName)
{
    var result   = {};
    var currName = currName || "";

    if(typeof(value)==='string' || typeof(value)==='boolean' || typeof(value)==='number' || value==null)
    {
        var splitName  = currName.split("_");
        var identifier = splitName.pop();

        console.log(identifier);
        if(identifier  == "TextCheck")
        {
            var boxFlag;
            value ? boxFlag = 'Y' : boxFlag = null;
            result[splitName.join('_').toUpperCase() + '_CHECKBOX'] = boxFlag;
            result[splitName.join('_').toUpperCase() + '_TEXT'    ] = value;
            return result;
        }
        if(identifier  == "RadioIndex")
        {
            if(value)
            {
                result[splitName.join('_').toUpperCase() + '_RADIOLABELS_' + value] = true;
            }
        }
        if(isDigit(identifier))                     //Check if last element of splitName is a radio button label
        {
            console.log("got a digit!");
            result[currName.toUpperCase()] = null;  //If so, use it to identify radio buttons, but throw out the description
            return result;
        }
        result[currName.toUpperCase()] = value;
        return result;
    }

    if(currName != "")
    {
        currName = currName + '_';
    }

    if(value instanceof Array)
    {
        for(var i = 0; i < value.length; i++)
        {
            mergeObjects(result, processKVFromObject(value[i], currName + i));
        }
    }
    else
    {
        for(key in value)
        {
            mergeObjects(result, processKVFromObject(value[key], currName + key));
        }
    }

    return result;
}

function flattenJSON(json)
{
    var keyValues = {};

    try
    {
        keyValues = processKVFromObject(json);
    }
    catch(err)
    {
        console.log("\nERROR in flattenJSON: " + err);
    }
    return keyValues;
}

Output:

"BILLINGSELFPAYOBJECT_AMMOUNT_TEXT": null,
"BILLINGSELFPAYOBJECT_CREDIT CARD_RADIOINDEX": null,
"BILLINGSELFPAYOBJECT_CREDIT CARD_RADIOLABELS_0": null,
"BILLINGSELFPAYOBJECT_CREDIT CARD_RADIOLABELS_1": null,
"BILLINGSELFPAYOBJECT_CREDIT CARD_RADIOLABELS_2": null,
"BILLINGSELFPAYOBJECT_CREDIT CARD_RADIOLABELS_3": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_DIAGNOSTIC_CHECKBOX": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_FAMILY HISTORY_CHECKBOX": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_KNOWN VARIANT_CHECKBOX": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_PRESYMPTOMATIC_CHECKBOX": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_OTHER (SPECIFY)_CHECKBOX": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_INDICATION_OTHER (SPECIFY)_TEXT": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_ICD-10 CODE(S)_TEXT": null,
"PATIENTQUESTIONNAIRE_GENERALQUESTIONS_CLINICAL DIAGNOSIS_TEXT": null,
\$\endgroup\$
2
\$\begingroup\$

Overall it's not too shabby. Does the job, certainly. Still, I have some notes:

  • Don't use brace-on-new-line style. Or, rather: I'd highly encourage you to use brace-on-same-line instead. The majority of JS uses same-line, so there's the conventionality of it, but there's one case where your current style can bite you, namely:

    function foo()
    {
      return
      {
        bar: 42
      };
    }
    

    JavaScript has a weird thing called automatic semi-colon insertion. Instead of complaining about semi-colons it thinks are missing, it tries to "fix" your code. So it'll see the return<end of line>, and say "there should probably be a semi-colon at the end there, so I'll insert it!". And thus the function returns undefined. Granted, these braces are for an object literal, not a code block, but the point is that brace-on-new-line can break things. Brace on same line won't.

  • Don't leave console.log calls in production code. They're fine for quick testing, but remove them when you're done.

  • Your mergeObjects() behaves differently from most implementations. Usually, the 2nd/source object's properties will overwrite those in the 1st/destination object, whereas your code won't do that. I'd stick with the conventional solution, so the code doesn't surprise anyone.
    Also, you should check hasOwnProperty() or use Object.keys to exclude prototypal properties.
    You can also do a variadic function that takes multiple "source" objects to merge into a single "destination" object:

    function merge(dest, source) {
      [].slice.call(arguments, 1).forEach(function (source) {
        Object.keys(source).forEach(function (key) {
          dest[key] = source[key];
        });
      });
      return dest;
    }
    

    The return isn't strictly necessary, as the function modifies dest, but it allows you to do things like var merged = merge({}, a, b, c), which will merge all the properties from the sources (a, b, and c) into a single new object, leaving the sources untouched.
    In this particular case, however, it might be best to simply throw an error if a property will be overwritten by the merge. It really shouldn't be possible, as all paths will be unique, but should it somehow happen, the upshot is that something from the JSON won't be included in the output. And I assume that's undesirable.

  • When using a ternary for assignments, only branch on the value, not the whole assignment. I.e. don't do this:

    value ? boxFlag = 'Y' : boxFlag = null; // not great
    

    but do this instead:

    boxFlag = value ? 'Y' : null; // better!
    
  • Instead of joining and splitting currName at each level of recursion, you could pass an array instead, and only join + upcase when you need the new key. It'd also eliminate the need for the isDigit() function, since you won't be converting between strings and numbers for array indices. Besides, isDigit is a little misleading - it'd return true for 3.1415 or "-42", despite those not being digits.

  • Finally, I'm not sure about this bit:

    result[currName.toUpperCase()] = null;  //If so, use it to identify radio buttons, but throw out the description
    

    Why discard the description? As far as I can tell, you'll just get output that says that radio button index 3 was selected, but you'll have no idea what that means, since *_RADIOLABEL_3 is just null.

Here's an alternative implmentation:

function flattenJSON(obj) {
  var result = [];

  // inner function that does all the work
  function recurse(path, obj) {

    // helper to append lines to the result
    function appendValue(key, value) {

      // another helper to join the path to the value
      const stringify = tail => path.concat(tail).join('_').toUpperCase();

      // attempt to match key as <name>_<type>
      const match = key.match(/^(.+?)_([^_]+)$/);

      // check if type is TextCheck, and if so handle it appropriately
      if (match && match[2] === 'TextCheck') {
        appendValue(`${match[1]}_CHECKBOX`, value !== null ? 'Y' : 'null');
        appendValue(`${match[1]}_TEXT`, value);
        return;
      }

      // any other types just get appended normally
      result.push(`${stringify(key)}: ${value}`);
    }

    // iterate through object
    Object.keys(obj).forEach(key => {
      const value = obj[key];

      if (typeof value === 'object' && value !== null) {
        // recurse into nested objects
        recurse(path.concat(key), value);
      } else if (value instanceof Array) {
        // handle arrays
        value.forEach((value, index) => appendValue(`${key}_${index}`, value));
      } else {
        // all other values
        appendValue(key, value);
      }
    });
  }

  // kick off the flattening
  recurse([], obj);

  return result.join('\n');
}

There's some ES6 syntax in since I find it nicer for something things (especially string interpolation), and the "footprint" of is reduced to a single function. All it's support/helper functions are nested within.

\$\endgroup\$
  • \$\begingroup\$ Thanks very much for your comments. They're well thought out and insightful. Regarding new brace on each line, it's specified by my work. I didn't know about the case where I return an object with different lined braces, and I will look out for it. The reason I don't overwrite existing keys in mergeObjects is because when I find a RadioIndex, I create a radio label and set it to true, as the associated RadioIndex will come afterward in the JSON and set it to null. I also don't utilize the RadioIndex description as I just need it to be set to checked for my html generation. \$\endgroup\$ – user1066886 Jan 23 '17 at 18:21

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.