1
\$\begingroup\$

I have written a simple wrapper for a 2D grid in JavaScript that allows me to resize the grid dynamically and I'd like to know if there's any way I can improve on my code.

Here is the code itself:

function Grid(width, height, defaultValue) {
  width = Math.max(0, width);
  height = Math.max(0, height);

  Object.defineProperties(this, {

    // The width of the grid. Changing this calls _updateInternalArraySize
    width: {
      get: function() {
        return width;
      },
      set: function(value) {
        if (value >= 0) {
          width = value;
          this._updateInternalArraySize();
        }
      }
    },

    // The height of the grid. Changing this calls _updateInternalArraySize
    height: {
      get: function() {
        return height;
      },
      set: function(value) {
        if (value >= 0) {
          height = value;
          this._updateInternalArraySize();
        }
      }
    },

    // The default value for empty cells
    defaultValue: {
      value: defaultValue
    },

    // The grid itself
    grid: {
      value: []
    }
  });

  // Initialize the grid
  this._updateInternalArraySize();
}

// Ensure x and y are in the correct range
Grid.prototype._validateCoordinates = function(x, y) {
  if (x < 0 || x >= this.width) {
    throw new RangeError('x must be in the range [0, ' + this.width + ')');
  }

  if (y < 0 || y > this.height) {
    throw new RangeError('y must be in the range [0, ' + this.height + ')');
  }
};

// Update the dimensions of the internal array
Grid.prototype._updateInternalArraySize = function() {
  var gridWidth = this.grid.length > 0 ? this.grid[0].length : 0;

  // Check if we actually need to do anything
  if (this.height === this.grid.length && this.width === gridWidth) {
    return;
  }

  // Increase row count to meet new height
  while (this.grid.length < this.height) {
    this.grid.push(Array(this.width).fill(this.defaultValue));
  }

  // Decrease row count to meet new height
  this.grid.length = this.height;

  for (var y = 0; y < this.height; y++) {

    // Increase column count to meet new width
    while (this.grid[y].length < this.width) {
      this.grid[y].push(this.defaultValue);
    }

    // Decrease column count to meet new width
    this.grid[y].length = this.width;
  }
};

// Get the value at the provided x and y
Grid.prototype.get = function(x, y) {
  this._validateCoordinates(x, y);

  return this.grid[y][x];
};

// Set the value for the provided x and y
Grid.prototype.set = function(x, y, value) {
  this._validateCoordinates(x, y);

  this.grid[y][x] = value;
};

// Convert the grid to a string
Grid.prototype.toString = function() {
  // Flatten all of the cells in the grid into a 1d string array
  var cells = this.grid.reduce(function(previous, current) {
    return previous.concat(current);
  }).map(function(value) {
    return String(value);
  });

  // Work out the maximum size of any cell
  var maximumCellSize = cells.reduce(function(previous, current) {
    return previous.length > current.length ? previous : current;
  }).length;

  var width = this.width;

  // Pad the cells and reduce them into a string
  return cells.map(function(value) {

    // Work out how long our padding should be and create it
    var length = Math.max(0, maximumCellSize - value.length),
        padding = Array(length).fill(' ').join('');

    return padding + value;
  }).reduce(function(previous, current, index) {
    var endOfRow = (index + 1) % width === 0,
        newLine = endOfRow && index !== cells.length - 1;

    // Add the current cell along with a space or newline depending on
    // whether or not we're at the end of a row and we should have a new line
    previous += current + (endOfRow ? '' : ' ') + (newLine ? '\n' : '');

    return previous;
  }, '');
};

And some examples of using it:

Resizing the grid

var grid = new Grid(5, 5, 0);

console.log(grid.toString());

// 0 0 0 0 0
// 0 0 0 0 0
// 0 0 0 0 0
// 0 0 0 0 0
// 0 0 0 0 0

grid.set(2, 2, 1);

console.log(grid.toString());

// 0 0 0 0 0
// 0 0 0 0 0
// 0 0 1 0 0
// 0 0 0 0 0
// 0 0 0 0 0

grid.width = 3;
grid.height = 3;

console.log(grid.toString());

// 0 0 0
// 0 0 0
// 0 0 1

grid.width = 10;
grid.height = 10;

console.log(grid.toString());

// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 1 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0
// 0 0 0 0 0 0 0 0 0 0

toString padding

var grid = new Grid(3, 3, 0);

console.log(grid.toString());

// 0 0 0
// 0 0 0
// 0 0 0

grid.set(1, 1, 1000);

console.log(grid.toString());

//    0    0    0
//    0 1000    0
//    0    0    0

One or two things I think I could improve:

  1. Maybe only pad individual columns instead of the whole table, as if one column is really wide we currently end up with them all being really wide.

  2. I don't know how I feel about the large defineProperties in the constructor, it looks like a bit of a mess but I don't know what I can do to remedy it.

  3. The toString method looks like a bit of a mess too but I don't know what I can do to fix it. One thing I don't like is how I've extracted this.width so I can access it from the reduce. I could bind the reduce method but I don't really like having to do that if there's another way around it.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Maybe only pad individual columns instead of the whole table, as if one column is really wide we currently end up with them all being really wide.

This is up to you. This is just a formatting issue.

Maybe only pad individual columns instead of the whole table, as if one column is really wide we currently end up with them all being really wide.

I usually recommend avoiding getters and setters via property changes. It is not explicit (unless you know in advance) that a property change can result to something else. Suggesting you expose the dimension changer as a real method.

The toString method looks like a bit of a mess

Seeing you use array.fill, this means you probably can do ES6. You can use arrow functions to trim off excess fat from the code.

For _updateInternalArraySize, you can just throw away the old arrays and generate new ones. Then copy over values that fit within the new grid. Since they're all 0 to begin with, we'll look for non-zeros. Complexity looks the same (a loop and a 2-level loop), but the code looks a bit more compact and straight-forward (create new, copy, throw old).

if (height === gridHeight && width === gridWidth) return;

var grid = Array(height).fill('').map(_ => Array(width).fill(0));

this.grid.forEach((row, rowIndex) => {
  row.forEach((cell, columnIndex) => {
    if(!(cell && rowIndex < height && columnIndex < width)) return;
    grid[rowIndex][columnIndex] = cell;
  });
});

this.grid = grid;

For your toString, you can easily collapse the grid to a 1D string array with reduce, and then again, reduce to find the longest one. You can use Math.max to easily get the bigger number among numbers instead of a ternary. For generating the string, you can use string.repeat instead of Array(length).fill(' ').join('') to create your padding and array.join to assemble the columns and rows.

// Look for the longest number
var pad = this.grid
              // Collapse every row to 1D
              .reduce((prev, current) => prev.concat(current))
              // Look for the largest digit
              .reduce((prev, current) => Math.max(`${current}`.length, prev), 0);

// Generate a string version of the grid
this.grid.map(row => {

  return row.map(cell => {

    // Pad each number when necessary
    return (' '.repeat(pad) + cell).slice(-pad);

  // Collapse columns into a string
  }).join('');

// Collapse rows into a string
}).join('\n');
\$\endgroup\$

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.