HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Two-dimensional array, modified one field at a time

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fieldarraytimemodifiedtwoonedimensional

Problem

Title says it all. I'm basically initializing a two-dimensional array and changing the fields one at a time.

It might be a good idea to do this OO.
A Grid class seems right, perhaps as a subclass of Array. So far I haven't been able to successfully port it. Pointers would probably be a good idea as well.

Note this is kind of a sandbox project at the moment. I'm a novice in JS and trying to wrap my head around how I can handle data efficiently (particularly memory efficient, CPU is not as relevant).

Partial reviews are also welcome.

Array.initialize = function(iRows, iColumns, cInit) {
    var array = [];
    for (var i = 0; i < iRows; ++i) {
        var columns = [];
        for (var j = 0; j < iColumns; ++j) {
            columns[j] = initial;
        }
       array[i] = columns;
    }
  return array;
}

Array.shoot = function(row, column, array) {
  array[row][column] = "H";
  return array;
}

var myGrid = Array.initialize(12, 12, "U");
myGrid = Array.shoot(5, 5, myGrid);
myGrid = Array.shoot(0, 4, myGrid);

console.log(myGrid);

Solution

A few things:

-
Someone's getting a head-start on Battleship, eh? :)

-
Don't modify native objects/prototypes. Array is the native JavaScript array constructor (a.k.a. "class"), and you're adding initialize and shoot methods to it. Don't.
Well, ok, you can extend native objects, but it's more hygienic to leave it alone. And in this case the methods you're adding are super-specific to your task, and not generic functionality, so that's a double "don't".

-
I'm pretty sure you have a bug: cInit isn't used for anything, but instead you have an previously undeclared (and therefor undefined) variable called initial. I'm guessing they were supposed to be one and the same.

Now, in terms of memory efficiency: Don't worry about it. JavaScript has automatic memory management and garbage collection, but it's buried deep in the runtime, so it's not something you can fiddle with. It also has dynamic typing, so it's kinda hard to even guess at memory consumption at times. For instance, the number 32 is 8 bytes long, since all numbers are 64-bit IEEE floating point values, yet 32 == "32" happens to be true due to dynamic typing/type-coercion. So the string "32" is for some purposes it's the same number, but it's probably only 2-3 bytes long. Maybe? I don't know. Because behind the scenes, all JS runtimes do a ton of voodoo, so in the end it's really hard to tell what's going on. Besides the runtime's own memory consumption is probably way beyond whatever data you're handling. The runtime might well trade memory for speed somewhere. And another runtime might do the opposite, but they'll both run your code the same.

Point is, memory just isn't a thing you really can worry about in JavaScript the same way you can worry about it in other languages. At least not on the same low level. Of course you can still run out of memory and things like that, but you'd have to try pretty hard. Especially with a simple little 2D array.

Aside: Don't bother with Hungarian notation either. Well, again, you can, but it's a lot of busywork when there's dynamic typing and type-coercion. You have parameters like iRows, which, yeah, should be an integer, but as mentioned all integers are just the Number type - which is really a float. And you could also pass in a numeric string and have it work. So it just gets confusing in a hurry.

Of course, you still want efficient data structures, but since the idea here is to construct a 2D array, well, you're probably best off using... a 2D array. So again, don't worry too much.

As for the code, you have the basics well in hand. As mentioned, you shouldn't be appending functions to the native Array constructor, but otherwise it's fine. Fixing that little thing you get:

function createArray(rows, columns, initialValue) {
  var array = [];
  for (var i = 0; i < rows; ++i) {
    var columns = [];
    for (var j = 0; j < columns; ++j) {
      columns[j] = initialValue;
    }
    array[i] = columns;
  }
  return array;
}

function shoot(row, column, array) {
  array[row][column] = "H";
  return array;
}


Still, I'd use push to append stuff to an array, rather than assigning things to an index.

But you said you might want a more OO-style implementation. However, sub-classing Array would be tricky. Especially as there are no "classes" in JavaScript, per se. It's easier to go with object composition; make your own constructor, and have your array be a property (aka member, aka instance variable) on the instances that constructor creates.

So let's make a Grid constructor and let's give it a shoot method:

function Grid(rows, columns) {
  var r, c, row;
  this.cells = []; // instance variable
  for(r = 0 ; r < rows ; r++) {
    row = [];
    for(c = 0 ; c < columns ; c++) {
      row.push("U");
    }
    this.cells.push(row);
  }
}

Grid.prototype.shoot = function (row, column) {
  this.cells[row][column] = "H";
};


Now, much like before, you can say:

var grid = new Grid(12, 12);
grid.shoot(5, 5);
grid.shoot(0, 4);

console.log(grid.cells); // notice you still have access to the "raw" array


Of course, you may want to do some bounds-checking in shoot to make sure the coordinates make sense, but otherwise... yeah, that's pretty much it.

Code Snippets

function createArray(rows, columns, initialValue) {
  var array = [];
  for (var i = 0; i < rows; ++i) {
    var columns = [];
    for (var j = 0; j < columns; ++j) {
      columns[j] = initialValue;
    }
    array[i] = columns;
  }
  return array;
}

function shoot(row, column, array) {
  array[row][column] = "H";
  return array;
}
function Grid(rows, columns) {
  var r, c, row;
  this.cells = []; // instance variable
  for(r = 0 ; r < rows ; r++) {
    row = [];
    for(c = 0 ; c < columns ; c++) {
      row.push("U");
    }
    this.cells.push(row);
  }
}

Grid.prototype.shoot = function (row, column) {
  this.cells[row][column] = "H";
};
var grid = new Grid(12, 12);
grid.shoot(5, 5);
grid.shoot(0, 4);

console.log(grid.cells); // notice you still have access to the "raw" array

Context

StackExchange Code Review Q#88914, answer score: 6

Revisions (0)

No revisions yet.