6
\$\begingroup\$

I am currently working on a project, and I just wrote a piece of it that requires a lot of array manipulation. Given a text input I want to expand it to fill the number of rows and columns I have. I worked out a solution, but I am concerned that I am creating too many arrays in my solution. I am looking for pointers on how I can make this code more efficient. I always get really lost when trying to refactor things like this.

function fillIn(data,numRows,numCols) {
  // First, split the data by newlines and tabs into an array of arrays.
  var dataArr = data.split('\n').map(function(row) { return row.split('\t'); });

  function fillRows(pRows){
    return [].concat.apply([], new Array(Math.floor(numRows / pRows.length)).fill(pRows));
  }

  function fillCols(pRows) {
    for (var i = 0; i < pRows.length; i++) {
      pRows[i] = [].concat.apply([], new Array(Math.floor(numCols / pRows[i].length)).fill(pRows[i]));
    }
    return pRows;
  }

  function fillSelection(pRows) {
    if (numRows > pRows.length) {
      pRows = fillRows(pRows);
    }
    if (numCols > pRows[0].length) {
      pRows = fillCols(pRows);
    }
    return pRows;
  }

  dataArr = fillSelection(dataArr);

  return dataArr;
}

console.log(fillIn('foo',2,2)); //[['foo','foo'],['foo','foo']]
console.log(fillIn('foo\tbar',4,2)); //[['foo','bar'],['foo', 'bar'],['foo','bar'],['foo','bar']]
console.log(fillIn('foo\nbar',2,4)); // [['foo','foo','foo','foo'],['bar','bar','bar','bar']]
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

ECMAScript 6

If you're able to use ECMAScript 6 then arrow functions could help clean up some of your code.

var dataArr = data.split('\n').map(row => row.split('\t'));

Unit Testing

You're logging the results, but it would be extremely helpful for your refactoring process to create some test cases for your function.

function test(input, expected) {
  var result = fillIn(input.data, input.numRows, input.numCols);

  function matrixEquals(m1, m2) {
    if (m1.length !== m2.length) return false;

    for (var r = 0; r < m1.length; r++) {
      if (m1[r].length !== m2[r].length) return false;

      for (var c = 0; c < m1[r].length; c++) {
        if (m1[r][c] !== m2[r][c]) return false;
      }
    }

    return true;
  }

  if (!matrixEquals(expected, result)) {
    throw "Test Failed for: input=" + JSON.stringify(input) +
      ", result=" + JSON.stringify(result);
  }
}

test({
  data: "foo\tbar",
  numRows: 2,
  numCols: 2
}, [
  ["foo", "bar"],
  ["foo", "bar"]
]);

test({
  data: "foo\tbar\nfat\tcat",
  numRows: 4,
  numCols: 4
}, [
  ["foo", "bar", "foo", "bar"],
  ["fat", "cat", "fat", "cat"],
  ["foo", "bar", "foo", "bar"],
  ["fat", "cat", "fat", "cat"]
]);

If you write your test cases well, you'll be able to tell if you broke your function simply by running the test cases.


Do you really even need to create all the arrays?

You could just extend the initial array directly:

function fillIn(data, numRows, numCols) {
  var matrix = data.split('\n').map(row => row.split('\t'));

  var missingRows = numRows - matrix.length;
  for (var r = 0; r < missingRows; r++) {
    matrix.push(matrix[r].slice());
  }

  var missingCols = numCols - matrix[0].length;
  for (var c = 0; c < missingCols; c++) {
    matrix.forEach(function(row) {
      row.push(row[c]);
    });
  }

  return matrix;
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the great answer. You are right I should have included better test cases. As a result of my poor testing, I think you missed one part of the problem. Values not only have to expand into the new range they also have to fit. \n i.e. test({ data: "foo\tbar", numRows: 3, numCols: 3 }, [ ["foo", "bar"], ["foo", "bar"] ]); I added a function that checks for that and now it works. function findBestFit(newLen,matrixLen) { var fit = newLen - matrixLen; while (fit % matrixLen !== 0) { fit -= 1; } return fit; } \$\endgroup\$
    – sharkpanic
    May 5, 2016 at 16:17

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.