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\$

    4 Answers 4

    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 < ndoesn'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\$
          2
          • \$\begingroup\$Sorry for the down-vote, but there are a few syntax errors and it doesn't work: jsfiddle.net/6y4shces\$\endgroup\$
            – tiffon
            CommentedDec 4, 2014 at 18:04
          • \$\begingroup\$My bad, I'm sorry about that. I've added some corrections.\$\endgroup\$CommentedDec 4, 2014 at 18:32

          Start asking to get answers

          Find the answer to your question by asking.

          Ask question

          Explore related questions

          See similar questions with these tags.