4
\$\begingroup\$

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;
      }
   })
}

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

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

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.