0
\$\begingroup\$

Could the following code be improved in terms of clarity, performance, maintainability, best practices etc?

JSONtab library

A tool to reversibly convert between an arbitrarily deep nested object and a tabular format, TSV by default.

// JSONtab library
// A tool to reversibly convert between an arbitrarily deep nested object and a
// tabular format, TSV by default.

const debug = require('debug')('app');
const set = require('lodash/set');
const isEqual = require('lodash/isEqual');

// test data
const testJSON = [
  {
    row: 0,
    'rec-0': {
      date: 20220121,
      tags: [
        'val-0',
      ],
    },
    'rec-1': {
      date: 20220116,
      url: 'https://example.com/a',
    },
  },
  {
    row: 1,
    'rec-0': {
      date: 20220116,
      url: 'https://example.com/b',
    },
    'rec-1': {
      tags: [
        'val-0',
        'val-1',
      ],
    },
  },
];

const testTSV = 'row\trec-0.date\trec-0.tags[0]\trec-1.date\trec-1.url\trec-0.url\trec-1.tags[0]\trec-1.tags[1]\n'
+ '0\t20220121\tval-0\t20220116\thttps://example.com/a\n'
+ '1\t20220116\t\t\t\thttps://example.com/b\tval-0\tval-1\n';

const calcSeparators = (columnLabel, sepValueString, colSep, lineSep) => {
// Return a string of column separators representing empty fields in the
// current row based on the position of columnLabel within the column header
// sepValueString : recursively built separated value table
// colSep : Column separator, default tab
// lineSep : Line separator, default newline
  const colSepChar = (colSep === undefined) ? '\t' : colSep;
  const lineSepChar = (lineSep === undefined) ? '\n' : lineSep;
  const colSepRegex = new RegExp(colSepChar, 'g');
  const lineSepRegex = new RegExp(`${lineSepChar}(.+)$`, 'g');
  const colHeader = sepValueString.split(lineSepChar)[0];
  const indColumnLabel = colHeader.indexOf(columnLabel);
  const labelNumber = (colHeader.slice(0, indColumnLabel).match(colSepRegex) || []).length;
  const currentRow = sepValueString.split(lineSepRegex)[1];
  const currColNumber = (currentRow.match(colSepRegex) || []).length;
  const addSeparators = labelNumber - currColNumber;

  let separators = '';
  for (let i = 0; i < addSeparators; i += 1) {
    separators = `${separators}${colSepChar}`;
  }
  return separators;
};

const JSONToTable = (JSONObject, pathElement, sepValueElement, colSep, lineSep) => {
// Return a separated value table from an arbitrary nested object JSONObject.
// pathElement : recursively built path for each value, used to populate column headers
// this preserves the object structure in order to reconstruct it later,
// see function tableToJSON
// sepValueElement : recursively built separated value table
// colSep : Column separator, default tab
// lineSep : Line separator, default newline
  let sepValueString = (sepValueElement === undefined) ? '' : sepValueElement;
  const colSepChar = (colSep === undefined) ? '\t' : colSep;
  const lineSepChar = (lineSep === undefined) ? '\n' : lineSep;
  Object.entries(JSONObject).forEach(([key, value]) => {
    let pathBuild;
    if (Array.isArray(JSONObject)) {
      pathBuild = (pathElement === undefined) ? `[${key}]` : `${pathElement}[${key}]`;
    } else {
      pathBuild = (pathElement === undefined) ? key : `${pathElement}.${key}`;
    }

    if (typeof value === 'object') {
      sepValueString = JSONToTable(value, pathBuild, sepValueString);
    } else {
      const columnLabel = pathBuild.split(/\.(.+)/)[1];
      const indEndHeader = sepValueString.indexOf(lineSepChar);
      const colHeader = sepValueString.slice(0, indEndHeader);
      const indColumnLabel = colHeader.indexOf(columnLabel);

      if (indColumnLabel === -1) {
        if (sepValueString.length) {
          // Add new column
          sepValueString = `${colHeader}${colSepChar}${columnLabel}${sepValueString.slice(indEndHeader)}`;
          const separators = calcSeparators(columnLabel, sepValueString, colSepChar);
          sepValueString = `${sepValueString}${separators}${value}`;
        } else {
          // Add first column
          sepValueString = `${columnLabel}${lineSepChar}`;
          sepValueString = `${sepValueString}${value}`;
        }
      } else if (sepValueString.slice(-1) === lineSepChar) {
        // Add first value in new row
        sepValueString = `${sepValueString}${value}`;
      } else {
        // Calculate column position and add subsequent value in row
        const separators = calcSeparators(columnLabel, sepValueString);
        sepValueString = `${sepValueString}${separators}${value}`;
      }
    }
  });
  if (pathElement !== undefined && !pathElement.includes('.')) {
    // End of row
    sepValueString = `${sepValueString}${lineSepChar}`;
  }
  return sepValueString;
};

const tableToJSON = (sepValueString, colSep, lineSep) => {
// Return an arbitrary nested object from a separate value table sepValueString
// The table column header contains paths which are used to reconstruct
// the object.
// colSep : Column separator, default tab
// lineSep : Line separator, default newline
  const colSepChar = (colSep === undefined) ? '\t' : colSep;
  const lineSepChar = (lineSep === undefined) ? '\n' : lineSep;
  const jsonArr = [];
  let rowObj = {};
  const rows = sepValueString.split(lineSepChar);
  let paths;
  rows.forEach((row, rowInd) => {
    if (!rowInd) {
      paths = row.split(colSepChar);
    } else {
      const values = row.split(colSepChar);
      values.forEach((value, columnInd) => {
        const pathElement = `${paths[columnInd]}`;
        if (value) {
          // remove quotes from value if possible
          const valueNum = value * 1;
          set(rowObj, pathElement, Number.isNaN(valueNum) ? value : valueNum);
        }
      });
      if (Object.keys(rowObj).length) {
        jsonArr.push(rowObj);
        rowObj = {};
      }
    }
  });
  return jsonArr;
};

// run tests
const sepValueString = JSONToTable(testJSON);
debug('matched test TSV', sepValueString === testTSV);
debug('matched test JSON', isEqual(tableToJSON(testTSV), testJSON));
debug('matched reverse JSON', isEqual(tableToJSON(sepValueString), testJSON));
\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

I am struggling a little bit at reading your code, so maybe it can be improved in terms of clarity. Some thoughts:

  • Pull the comment about what a function does above the method signature. It can be a headline of what the function is going to do. It can focus on the goal of the function (not the implementation, which belongs inside the function). For me, these were a little hard to understand, so you may want to work on them a little bit.

  • Tighten up the terminology. I don't understand exactly what a "tabular format" or "separated value table" is, and what the difference is. There's also a bit of messiness in "JSON" vs. "an arbitrary nested object JSONObject" vs. "JSONObject"-- just pick one and stick with it.

  • Think about how the reader is going read through your code, and what story they are going to find. The first few lines define some test objects, but then these disappear, and don't re-appear until the very end. You're asking your reader to "push these onto the stack" while they read the rest of your code. Perhaps you can just put the test lines right there with the test inputs. See if you can sequence of methods to lay out the structure.

  • Some of the names are a mouthful. I think the name sepValueString could probably just be lines. Then code would read const colHeader = lines.split(lineSepChar)[0].

  • You have lots of variables. Try to get rid of the some. For example, you have both colSep and colSepChar. The line const colSepChar = (colSep === undefined) ? '\t' : colSep; could just be if (colSep === undefined) colSep = '\t'. Or even better, use a default parameter for this, which gets rid of that line completely.

  • Consider introducing a small structure to hold are the configurations/options: in this case, the separator characters. Then the calls can be more simply foo(text, options), where options is an object with a colSep, lineSep, etc. Method calls will be easier to understand. You can write shared methods that initialize this object or fill in defaults.

  • Some of your methods are really long. Consider factoring out pieces of them. Maybe a method that processes lines would work, but I'm not sure.

Hope this helps!

\$\endgroup\$
1
  • \$\begingroup\$ Yes, it does help a lot! I didn't know js had default parameters. I have tried to improve on all the points you mentioned and updated the code. Further comments / criticism welcome \$\endgroup\$ Feb 10, 2022 at 17:32
2
\$\begingroup\$

Considering JSON is derived from JavaScript, I was surprised that I could not find an existing JavaScript library which can reversibly convert it to common formats such as CSV. Consequently, I developed JSONDSV below

Based on the good suggestions of @ndp, I made the following changes -

  • moved the function descriptions to above the signatures
  • standardized terminology to delimiter-separated values (DSV) to avoid confusion
  • moved test related data and code to the bottom
  • renamed the variable sepValueString to dsv
  • eliminated some lines of code and variables. I could have reduced the number of constants in calcSeparators by combining them like const addSeparators = (colHeader.slice(0, indColumnLabel).match(colSepRegex) || []).length - (currentRow.match(colSepRegex) || []).length ... but that would reduce readability, agree?
  • introduced options default parameter
  • while I prefer having less variables and code, and consequently less bugs, I could not find a way to reduce them further without impacting readability, can you?

revised code-

// JSONDSV library
// ===============
// A tool to reversibly convert between an arbitrarily deep nested
// array and a configurable delimiter-separated values (DSV) format
// such as tab (TSV), comma (CSV), pipe, etc. Reversibility is achieved
// by populating the header row of the DSV with paths from the array.

// Use case
// --------
// For arrays which are fairly tabular, i.e. composed of records
// with similar structure, converting to DSV format facilates
// processing with spreadhsheets, databases or other common tools.
// In addition, the structure of the array can be altered by simply
// editing the header row and then converting back to an array.

// Tables with many columns of sparse data (i.e lots of blank cells)
// can be converted to a more compact array format since empty values
// are not encoded.

// Examples and testing
// --------------------
// See below for an example of a nested array (testArray), its
// equivalent DSV formats and a simple test runner. With Node.js,
// the tests can be executed with DEBUG=app node jsondsv.js

const debug = require('debug')('app');
const set = require('lodash/set');
const isEqual = require('lodash/isEqual');

// Helper function calcSeparators returns a string of column
// separators representing empty fields in the last row of the DSV
// based on the position of columnLabel within the header row
const calcSeparators = (
  columnLabel,
  dsv,
  options = { colSep: '\t', lineSep: '\n' },
) => {
// columnLabel : currently processed column
// DSV : recursively built delimiter-separated values
// options.colSep : Column separator, default tab
// options.lineSep : Line separator, default newline

  const colSepRegex = new RegExp(options.colSep, 'g');
  const lineSepRegex = new RegExp(`${options.lineSep}(.+)$`, 'g');
  const headerRow = dsv.split(options.lineSep)[0];
  const indColumnLabel = headerRow.indexOf(columnLabel);
  const labelNumber = (
    headerRow.slice(0, indColumnLabel).match(colSepRegex) || []).length;
  const splitHeader = dsv.split(lineSepRegex);
  const lastRow = (splitHeader.length > 1) ? splitHeader[1] : '';
  const currColNumber = (lastRow.match(colSepRegex) || []).length;
  const addSeparators = labelNumber - currColNumber;

  let separators = '';
  for (let i = 0; i < addSeparators; i += 1) {
    separators = `${separators}${options.colSep}`;
  }
  return separators;
};

// Function arrayToDSV returns a DSV from an arbitrarily deep nested
// array input.
const arrayToDSV = (
  nestedArray,
  options = { colSep: '\t', lineSep: '\n' },
  internal = { path: '', dsv: '' },
) => {
// nestedArray : arbitrarily deep nested array input
// internal.path : internal recursively built path for each value
//   used to populate header row
// internal.dsv : internal recursively built delimiter-separated values
// options.colSep : Column separator, default tab
// options.lineSep : Line separator, default newline
  let dsv = (internal.dsv.length) ? internal.dsv : options.lineSep;
  Object.entries(nestedArray).forEach(([key, value]) => {
    const path = (Array.isArray(nestedArray))
      ? `${internal.path}[${key}]`
      : `${internal.path}.${key}`;
    if (typeof value === 'object') {
      dsv = arrayToDSV(value, options, { path, dsv });
    } else {
      const columnLabel = path.split(/\.(.+)/)[1];
      const indEndHeader = dsv.indexOf(options.lineSep);
      const headerRow = dsv.slice(0, indEndHeader);
      const indColumnLabel = headerRow.indexOf(columnLabel);
      if (indColumnLabel === -1) {
        // Add new column
        dsv = (headerRow.length)
          ? `${headerRow}${options.colSep}${columnLabel}${dsv.slice(indEndHeader)}`
          : `${columnLabel}${dsv.slice(indEndHeader)}`;
      }
      // Add value
      const separators = calcSeparators(columnLabel, dsv, options);
      dsv = `${dsv}${separators}${value}`;
    }
    if (!path.includes('.')) {
    // End of row
      dsv = `${dsv}${options.lineSep}`;
    }
  });
  return dsv;
};

// Function DSVToArray returns an arbitrarily deep nested array
// with paths in the header row which are used to reconstruct the array
const DSVToArray = (dsv, options = { colSep: '\t', lineSep: '\n' }) => {
// dsv : input delimiter-separated values string
// options.colSep : Column separator, default tab
// options.lineSep : Line separator, default newline
  const jsonArr = [];
  let rowObj = {};
  const rows = dsv.split(options.lineSep);
  let paths;
  rows.forEach((row, rowInd) => {
    if (!rowInd) {
      // Extract paths from header row of the dsv (first row)
      paths = row.split(options.colSep);
    } else {
      const values = row.split(options.colSep);
      values.forEach((value, columnInd) => {
        const path = `${paths[columnInd]}`;
        if (value) {
          // remove quotes from value if possible
          const valueNum = value * 1;
          set(rowObj, path, Number.isNaN(valueNum) ? value : valueNum);
        }
      });
      if (Object.keys(rowObj).length) {
        jsonArr.push(rowObj);
        rowObj = {};
      }
    }
  });
  return jsonArr;
};

// test data
const testArray = [
  {
    row: 0,
    'rec-0': {
      date: 20220121,
      tags: [
        'val-0',
      ],
    },
    'rec-1': {
      date: 20220116,
      url: 'https://example.com/a',
    },
  },
  {
    row: 1,
    'rec-0': {
      date: 20220116,
      url: 'https://example.com/b',
    },
    'rec-1': {
      tags: [
        'val-0',
        'val-1',
      ],
    },
  },
];

const testTSV = 'row\trec-0.date\trec-0.tags[0]\trec-1.date\trec-1.url\trec-0.url\trec-1.tags[0]\trec-1.tags[1]\n'
+ '0\t20220121\tval-0\t20220116\thttps://example.com/a\n'
+ '1\t20220116\t\t\t\thttps://example.com/b\tval-0\tval-1\n';
const testCSV = testTSV.replace(/\t/g, ',');

// run TSV tests
const tsv = arrayToDSV(testArray);
debug((tsv === testTSV), 'matched array converted to TSV');
const arrayTSV = DSVToArray(testTSV);
debug(isEqual(arrayTSV, testArray), 'matched converted TSV to array');
debug(isEqual(DSVToArray(tsv), testArray), 'matched array to TSV and reverse');
debug(isEqual(arrayToDSV(arrayTSV), testTSV), 'matched TSV to array and reverse');

// run CSV tests
const csv = arrayToDSV(testArray, { colSep: ',', lineSep: '\n' });
debug((csv === testCSV), 'matched array converted to CSV');
const arrayCSV = DSVToArray(testCSV, { colSep: ',', lineSep: '\n' });
debug(isEqual(arrayCSV, testArray), 'matched converted CSV to array');
debug(isEqual(DSVToArray(csv, { colSep: ',', lineSep: '\n' }), testArray), 'matched array to CSV and reverse');
debug(isEqual(arrayToDSV(arrayCSV, { colSep: ',', lineSep: '\n' }), testCSV), 'matched CSV to array and reverse');
\$\endgroup\$
0

Your Answer

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

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