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

    1 Answer 1

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

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.