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.