current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I need to create a \${n} \times {n}\$ multidimensional array filled with default values.

Right now, I'm using this code:

//--- how many columns?
var colNumber = 10;
//--- how many rows?
var rowNumber = 20;

//--- default value
var defaultValue = "default";

var defRow = [];
var defaultArray = [];

for(var x = 0; x < colNumber; x++){
    defRow.push(defaultValue);   
}
for(var x = 0; x < rowNumber; x++){
    defaultArray.push(defRow);
}
console.log(defaultArray);

find fiddle here

Is there any better approach to do this?

share|improve this question

3 Answers 3

up vote 6 down vote accepted

Yes, there's a better approach. The problem you have here is that each row is the same value, so a change to one row will be seen in all rows. For example, I added the following to your code to illustrate this:

console.log(JSON.stringify(defaultArray));
defaultArray[5][5] = "Fubar";
console.log(JSON.stringify(defaultArray));

This should change the value in the middle of the 'matrix', but instead the result is:

"[["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"],
["default","default","default","default","default","Fubar","default","default","default","default"]]"

You need to have a fresh array instance for each row.

Even though it is mundane, the logical way to do it is:

var defaultArray = [];

for(var x = 0; x < rowNumber; x++){
    var defRow = [];
    for(var y = 0; y < colNumber; y++){
        defRow.push(defaultValue);   
    }
    defaultArray.push(defRow);
}

Edit: Based on the benchmarks for my browser (Firefox), a decrementing while-loop is much faster than alternatives. I would use:

var defaultArray = [];

for(var row = rowNumber - 1; row >= 0; row--){
    var defRow = [];
    for(var col = colNumber - 1; col >= 0; col--){
        defRow[col] = defaultValue;   
    }
    defaultArray[row] = defRow);
}

By working backwards over the array you reduce the amount of space allocations that happen as the array grows. Here's the results of that benchmark for me:

Benchmarks

share|improve this answer
    
well noted, I totally forgot that. Do you think defRow.slice(0) would be better to clone the entire row in one time instead of using the second nested for loop? –  BeNdErR Dec 9 '14 at 12:57
    
See This SO post which has mixed recommendations, including defRow.slice(), defRow.slice(0), and a while loop counting backwards. Based on the benchmarks here the while-loop is much, much faster for me. –  rolfl Dec 9 '14 at 13:20
1  
Fixed the benchmark. The array b wasn't initialised in the test, so it was overwriting properties that already existed instead of adding properties. Now the while loop gets a reasonable result. jsperf.com/new-array-vs-splice-vs-slice/60 –  Guffa Dec 9 '14 at 13:52
up vote 5 down vote

From a once over:

  • I tend to call colNumber -> colCount or columns
  • It is considered better practice to group your variables in 1 comma-chained var
  • You declared var x twice, this is not needed, and does most likely not what you think it does.
  • My nitpicker side cringes to see defaultArray, defaultValue and defRow
  • Your comments are too much, comments must be worth their space, prime offender:
    //--- default value
    var defaultValue = "default";

My counter-proposal woul be:

var columnCount = 10,
    rowCount = 20,
    column, row;

var defaultArray = [],
    defaultValue = "default",
    defaultRow = [];

//Not the most fastest way, I defer to Rolf for that part
for(row = 0; row < rowCount ; row++){
    defaultRow = [];
    for(column = 0; column < columnCount ; column++){
        defaultRow[column] = defaultValue;   
    }
    defaultArray[row] = defaultRow;
}

console.log(defaultArray);
share|improve this answer
    
thank you for your suggestion! :) –  BeNdErR Dec 9 '14 at 14:09
up vote 3 down vote

You can use a function that creates an array, then just use it to create an array for each item in the array:

function createArray(len, val) {
  var a = [];
  var func = typeof val === 'function';
  for (var i = 0; i < len; i++) {
    a[i] = func ? val() : val;
  }
  return a;
}

var defaultArray = createArray(20, createArray.bind(this, 10, "default"));

// Display in StackExchange snippet
document.write(JSON.stringify(defaultArray));

Note: The bind method isn't supported in IE8, but you can just use a function expression instead if you need to support it. The jQuery library also has a replacement ($.proxy), and I guess many other libraries have one too.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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