3
\$\begingroup\$

I have an array of objects. Whatever may be the size of this array, I need to ensure the size of the array as n using a default object.

 function fillRemaining(arr, n) {
        var defaultObject = {'id' : 0, 'name' : ''};
        var temp = 0;
        while(arr.length + temp < n ) {
           var template = $.extend({}, defaultObject);
           template.id = new Date().getTime();
           arr.push(defaultObject);
           temp++;
        }
    }

Is there a better method of doing it?

\$\endgroup\$
2
\$\begingroup\$

I see a few ways this can be improved.

First, you are creating the template variable as a copy of the defaultObject, but then you are inserting defaultObject rather than template. I am going to assume this is a bug.

I would also say that there is not a big reason why you should have a defaultObject, then create and modify a copy to add. Just create objects on the fly.

Second, you are using a while loop, but constructing it like a for loop (loop variable, incremented every iteration). You actually don't even use the loop variable, so it can be removed completely. There is also a second potential bug where you are checking against arr.length + temp in your conditional, but arr.length increases every time you add an element to the array, so you are counting by 2's instead of by 1's.

Lastly, the name fillRemaining does not clearly describe what the function is supposed to do. Since it is padding onto the end of the array, perhaps padArrayWithDefault or something similar might be more descriptive.

Here's what I would suggest:

function padArrayWithDefault(arr, n) {
    while(arr.length < n) {
        arr.push({
            "id": new Date().getTime(),
            "name": ""
        });
    }
}
\$\endgroup\$
1
\$\begingroup\$

This is just a generic approach, using a callback to define what the "default" value should be:

function padArray(array, length, callback) {
  while(array.length < length) {
    array.push(callback());
  }
}

In your case, you'd use it like so:

padArray(someArray, 42, function () {
  return { id: Date.now(), name: "" };
});

Note: Date.now() is supported in modern runtimes, but not in some older ones (click "show obsolete browsers" to see). But you can always use (new Date).getTime() like you are now.


In terms of code review:

  • The arr.length + temp < n doesn't actually work, as far as I can tell. When you do arr.push(), the array's length increases, but you also increment temp. So your loop will run half as many times as you intend.

  • Your code - like mine above - may trigger an infinite loop if the arr argument (array in my code) isn't an array. E.g. if arr.length is undefined the while-condition will never be true. So you may want to check for that.

  • You're using a default object that you're then "cloning" using extend. Two things: 1) given how simple it is, just write it like above instead of cloning, 2) extend doesn't do a "deep" clone, so if any of the properties are themselves mutable objects, there'll just be references to the same instance copied all over.

\$\endgroup\$
0
\$\begingroup\$

I don't know if this is better, but might be more flexible to have a parameter for a factory function that creates the objects you're filling the Array with.

I also added from and to parameters so subsections can be replaced instead of just the last n elements.

function replace(arr, factory, from, to) {
    var i = from;
    if (!from) {
        i = 0;
    } else if (from < 0) {
        i = arr.length + from;
    }
    if (to < 0) {
        to = arr.length + to;
    } else if (to == null) {
        to = arr.length;
    }
    for (; i < to; i++) {
        arr[i] = factory();
    }
}

Using it:

var arr;

// making test arrays
function mkArr() {
    return 'a b c d e f g h i'.split(' ');
}

// the factory function for the new values
function makeDefault() {
    return {
        id: +new Date(),
        name: ''
    };
}

// to replace all values in `arr`
arr = mkArr();
replace(arr, makeDefault);

// to replace the last 3 values
arr = mkArr();
replace(arr, makeDefault, -3);

// everything but the first two values
arr = mkArr();
replace(arr, makeDefault, 2);

// replace the second, third and fourth values
arr = mkArr();
replace(arr, makeDefault, 1, 4);

A jsfiddle: http://jsfiddle.net/atca3xpy/

\$\endgroup\$
-1
\$\begingroup\$

Im not sure if this is the proper forum for this question. But if I understand correctly, you're trying to create an array that is prefilled with a default object.

If this is the case:

You could simplify by calling the Array constructor with n and then just map over it.

function Fill(n, _default) {
   return Array.apply(null,Array(n)).map(function (val) {  
     return _default || { };
   });
}
\$\endgroup\$
  • \$\begingroup\$ Sorry for the down-vote, but there are a few syntax errors and it doesn't work: jsfiddle.net/6y4shces \$\endgroup\$ – tiffon Dec 4 '14 at 18:04
  • \$\begingroup\$ My bad, I'm sorry about that. I've added some corrections. \$\endgroup\$ – jhenriquez Dec 4 '14 at 18:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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