Inserting Elements Into a Sorted Array

Question

Very new to JS (wrote my "Hello World" around 3 days ago), but not to programming in general. My documentation is inspired by the approach I followed for Python (since I haven't learned JS documentation conventions yet).

My Code

"use strict";

/*
   Inserts elements from a given source array in a specified target array in sorted order. Modifies the target array in place.

   Args:
      * `source` (Array): Source array. The array is assumed to be already sorted.
      Sorting is not done, as a custom sort function may be desired, and checking for sorting is not done as doing so would 
      greatly diminish the performance benefits of working with already sorted data.
      * `target` (Array): Target array. The array is assumed to be already sorted (for similar reasons as above).
      * `cmp` (function): Comparison function (predicate). 
         - Defaults to `<` operator. 
         - Args:
            `a`.
            `b`.
         - Returns (boolean): Indicates if `a < b`.
            It is important that the predicate indiates `a < b` and not `a <= b` so that the sort is stable.
*/
const sortedInsert = function(source, target, cmp = (a, b) => a < b)
{
   let insertPosition = 0;
   source.forEach(itm => {
      let insertAtEnd = true;
      for(let i = insertPosition; i < target.length; i++)
      {
         if(cmp(itm, target[i]))
         {
            target.splice(i, 0, itm);
            insertPosition = i;
            insertAtEnd = false;
            break;
         }
      }
      if (insertAtEnd)
      {
         target.splice(target.length, 0, itm);
         insertPosition = target.length;
      }
   })
}

Answer 1

Good things:

• There is a very descriptive docblock above the function that documents the arguments.
• const is used for things that don't change, whereas let is used for values that can be re-assigned.

Suggestions

• When inserting at the end, just use Array.push() instead of Array.splice(). This not only simplifies the syntax but also should be faster (see this jsperf).
• For performance reasons it would be wise to use a for...of loop instead of a forEach() to iterate over the items in source - especially for large arrays. Functional programming is nice but it has drawbacks - especially when performance is concerned. Consider that each iteration has a call to cmp so calling an extra function for each element in source could lead to a lot of extra overhead. With such a change the variable insertAtEnd could likely be removed if a label outside the outer for loop was added and that label was used with a continue statement instead of the break.