Improving a function from using 1-D arrays to 2-D arrays as argument

The problem:

I have a function that writes an array it takes as argument to a div, in a single line.

Example:
writeToOutput([1,2,3]) outputs
1,2,3

The aim is to improve this function, to be able to take an array of arguments, and display them on several lines

Example: writeToOutput([[1,2,3],[4,5,6]]) outputs

1,2,3
4,5,6

An obvious solution is to use writeToOutput([[1,2,3]]) to replicate the previous results, however, the function call with a 1-D array is already used by several scripts, and I would like them to keep working.

The code:

function writeToOutput(values, outputName) {
  var outputDiv = document.getElementById(outputName)
  var outputText = "";

  if (!Array.isArray(values[0])) {
    values = [values]
  }

  var valuesSubArray;
  for (var i = 0; i < values.length; i++) {
    valuesSubArray = values[i];
    outputText = "";
    for (var j = 0; j < valuesSubArray.length; j++) {
      outputText += valuesSubArray[j]
    }
    if (i > 0) {
      outputDiv.innerHTML += "</br>";
    }
    outputDiv.innerHTML += outputText;
  }
}

What I am interested in:

As you may have guessed, I don’t really care about writing numbers to a div, but I have a legacy function that takes a 1-D array, and I would like it to deal with 2-D arrays without breaking existing implementation.

I am interesting in figuring out if the trick with Array.isArray(values[0]) is the best way to do this, or if there is more performant, more widely used, etc. ways to do this. (The project is using jQuery).

Altought this stub has been made to sugarcoat the 1-D -> 2-D array, I am eager to learn, and if there is an obvious improvement to be made, please let me know!

Regarding your main question

From my point of view, yes, your current solution to allow legacy 1-D arrays to work with your new version is the best one I can think.
Rather funny: I just proposed a close solution in a PHP context question, less than 24 hours ago!

Another obvious method might be to keep the legacy function as is, and to write an almost identical one (simply expecting only 2-D arrays) with an alternative name.
But it doesn’t seem better at all, notably because in case of changes regarding the core job they do, these two functions should be both updated!

Now since you asked for other possible improvements

I know that your example is not a real one, but since you implemented it “seriously” I propose you some points you might consider.

Performance

for (var i = 0; i < values.length; i++) {
  // ...
}

With the above code (your outer loop), values.length is evaluated for each step while looping, that may become significative when length is high.

A well known best practice is to avoid it, using this instead:

for (var i = 0, n = values.length; i < n; i++) {
  // ...
}

This way, values.length is evaluated only once.

Simplification

Anyway, instead of the above code you can use this alternative construction, resulting in a simpler code:

values.forEach(function(valuesSubArray, i) {
  // ...
});

NOTE: with this construction, you can also drop the previous declaration var valuesSubArray;.

Again simplification, and probably also performance

outputText = "";
for (var j = 0; j < valuesSubArray.length; j++) {
  outputText += valuesSubArray[j]
}

The above code (your inner loop) can be replaced by this simple statement:

outputText = valuesSubArray.join('');

NOTE: this results in 123 for a sub-array like [1, 2, 3], like does your outputText += valuesSubArray[j]. If you rather want to get 1,2,3 as stated by the initial explanation in your OP, you must use outputText = valuesSubArray.join(','); instead.

More simplification

At a higher level we can observe that the whole desired result outputDiv.innerHTML structure is the same as the outputText‘s one`:

• each element is the current outputText
• the separator is <br /> instead of a comma

So we can use join('<br />') at this level, applying it to an array of the generated outputTexts. This array can be obtained using reduce():

values.reduce(
  function(result, valuesSubArray) {
    result.push(valuesSubArray.join(','));
    return result;
  },
  []
);

NOTE: we don’t need using i any longer, nor previously declaring outputText.

Finally

We can completely drop any variable declaration, and achieve the whole job with a “one-liner”:

function writeToOutput(values, outputName) {
  if (!Array.isArray(values[0])) {
    values = [values]
  }
  
  document.getElementById(outputName).innerHTML = 
    values.reduce(
      function(result, valuesSubArray) {
        result.push(valuesSubArray.join(','));
        return result;
      },
      []
    )
    .join('<br />');
}

writeToOutput([[1, 2, 3], [4, 5, 6]], 'outputs');

<div id="outputs"></div>

Since you are looking for performance, using a functional method like array.forEach or array.reduce is not advised. for will always be the fastest way as these functions accept a callback to perform processing, and that adds to performance.

That said, you can still use functional approach in your code.

Suggestions:

• Try to create more generic functions. This will ensure you use your code more. It will also help you to create a function that only focuses on the business logic.
• Try to avoid innerHTML. It rewrites the HTML so any handlers attached will get lost.
• Also, DOM manipulations are very expensive. You should try to keep them at minimum. So updating innerHTML in loop is a bad idea. Instead, append it to a local string and set it once and perform a bulk operation.
• Try to avoid <br/>. Its not a good practice. Instead create a block element like div or p or set display: block to the element.
• Try to use inbuilt methods:

for (var j = 0; j < valuesSubArray.length; j++) {
  outputText += valuesSubArray[j]
}

This loop is unnecessary. If all you want is to get a concatinated string, use functions like array.join(delimiter). By default, array.toString will join the elements by comma.

function writeToOutput(values, selector) {
  var outputDiv = document.getElementById(selector);
  for (var i = 0, len = values.length; i < len; i++) {
    outputDiv.append(createRow(processArray(values[i])));
  }
}

function createRow(str) {
  var div = document.createElement('div');
  div.innerHTML = str;
  return div;
}

function processArray(array, callback) {
  if (callback) {
    if (typeof array === 'object') {
      var temp = '';
      for (var k in array) {
        temp += callback(array[k]);
      }
      return temp;
    } else {
      return callback(array)
    }
  } else {
    return array.toString();
  }
}

writeToOutput([
  [1, 2, 3],
  [4, 5, 6]
], 'outputs')

<div id="outputs"></div>

Reference:

• Is something wrong with BR
• Why is it suggested to avoid innerHTML
• Alternatives to avoid innerHTML

Use the inbuilt toString() coercion.

Javascript is a loosely typed language. When you use two objects that are not the same type javascript will convert one of them to match the type of the other, if possible. This is called type coercion.

console.log(1 * "3"); // outputs 3 the string "3" is coerced to a Number
console.log("A number " + 3); //the 3 is coerced to a String
console.log("A array " + [1,2,3]); // the array coerced to a String "1,2,3"

When an object (an array is an object) needs to be converted to a String Javascript calls its toString() method. For the array the toString method does the same as the Array.join(",") function. Each item in the array is also converted to a string, if needed, and using its toString.

So an array of arrays will be converted to a string by just assigning the array to a string. const str = ""; str += [[1,2,3],[4,5,6]; console.log(str); outputs "1,2,3,4,5,6"

This means all the work your function needs to do is mostly done for you. You just need to make sure that you add the line break tag "<br>" at the end of each array. For that you can use the Array.join function directly.

Delimiting the output.

There remains one problem. If you call [1,2,3].join("<br>") it will put a break between each item. So you need to still have the test.

Creating a second array is overkill as all you need is what delimiter to use. This can be done with a simple expression.

var delimiter = Array.isArray(array[0]) ? "<br>" : ",";

And then join the array using the delimiter

const str = array.join(delimiter);

_{Note: I assume that the test you use is accepted as the way to determine if the array contains arrays.}

Test the input

You can not be sure that the id of the element is correct. What you do in that case is upto the design specs for the function. I will assume that your function is not meant to generate any type of error. So you need to check that the id gets an element.

As we are now calling an array method directly Array.join(","); the function will throw an error is the passed argument does not contain an array. So you need to check that as well.

A rewrite.

function arrayToString(arr, outputId) {
  const div = document.getElementById(outputId);
  if (div !== null) {
      if (Array.isArray(arr)) {  
          const delimiter = Array.isArray(arr[0]) ? "<br>" : ",";
          div.innerHTML += arr.join(delimiter) + "<br>";
      } else {
          div.innerHTML += arr + "<br>";
      }
  }
}
arrayToString([1,2,3,4,5], "test");
arrayToString(["A","B","C","D","E"], "test");
arrayToString([["A","B"],["C","D","E"]], "test");

div { border : 1px solid black }

<div id="test"></div>

And just to clarify Javascript arrays.

Javascript only has 1D arrays.

In Javascript there are really only 1D arrays. We can simulate 2D to infinite D arrays by adding arrays to each item. But you can not guarantee that each item will be another array, or if it is an array, what and how many items it contains.

const a = [1,2,3,4];
const b = [[1,2,3],[4,5,6]];
const c = [[1,2,3],4,5,6];

a is a flat array. b is an array of arrays and is not a 2D array. c is a mixed content array.

const d = [a,2,3,4];
a[0] = d;

d is an array holding the array a and by adding to a[0] the array d I have created an cyclic reference. If you were to use the dimensional description it would be a infinite dimensioned array, which is clearly impossible (no memory store is big enough) but still d[[[[[[[[[[[[[[[[[[[[1]]]]]]]]]]]]]]]]]]]] === 2 works. I have never tested how far you can nest the [] but there is a limit somewhere.