Tidy sorting code & add tests

Author: MetRonnie

src/components/cylc/common/sort.js

@@ -70,27 +70,26 @@ export const DEFAULT_COMPARATOR = (left, right) => {
  * @param {Object[]} array - list of string values, or of objects with string values
  * @param {Object} value - a value to be inserted in the list, or an object wrapping the value (see iteratee)
  * @param {SortedIndexByIteratee=} iteratee - an optional function used to return the value of the element of the list}
- * @param {SortedIndexByComparator=} comparator - function used to compare the newValue with otherValues in the list
+ * @param {{comparator: SortedIndexByComparator, reverse: boolean}=} options - an optional object with a comparator function and a reverse flag
  * @return {number} - sorted index
  */
-export function sortedIndexBy (array, value, iteratee, options = {}) {
-  // comparator, reverse = false) {
+export function sortedIndexBy (array, value, iteratee = (x) => x, options = {}) {
   if (array.length === 0) {
     return 0
   }
-  // If given a function, use it. Otherwise, simply use identity function.
-  const iterateeFunction = iteratee || ((value) => value)
   // If given a function, use it. Otherwise, simply use locale sort with numeric enabled
-  const comparatorFunction = options.comparator || ((leftObject, leftValue, rightObject, rightValue) => DEFAULT_COMPARATOR(leftValue, rightValue))
+  const comparator = options.comparator || (
+    (leftObject, leftValue, rightObject, rightValue) => DEFAULT_COMPARATOR(leftValue, rightValue)
+  )
   let low = 0
   let high = array.length
 
-  const newValue = iterateeFunction(value)
+  const newValue = iteratee(value)
 
   while (low < high) {
     const mid = Math.floor((low + high) / 2)
-    const midValue = iterateeFunction(array[mid])
-    let higher = comparatorFunction(value, newValue, array[mid], midValue)
+    const midValue = iteratee(array[mid])
+    let higher = comparator(value, newValue, array[mid], midValue)
     if (options.reverse) {
       higher = higher * -1
     }

src/store/workflows.module.js

@@ -124,7 +124,7 @@ function getIndex (state, id) {
   return state.cylcTree.$index[id]
 }
 
-/* Add a child node under a parent Node */
+/** Add a child node under a parent Node */
 function addChild (parentNode, childNode) {
   // determine which list to add this node to
   // console.log(`$t ++ ${childNode.id}`)
@@ -144,20 +144,12 @@ function addChild (parentNode, childNode) {
   }
 
   // insert the child preserving sort order
-  let comparator
-  if (['cycle', 'family'].includes(parentNode.type)) {
-    // sort by type, then name
-    // (this makes families sort before tasks in the graph)
-    comparator = (n) => `${n.type}-${n.name}`
-  } else {
-    // sort by name
-    comparator = (n) => n.name
-  }
+  const iteratee = (n) => `${n.type}-${n.name}` // (this makes families sort before tasks in the tree)
   const reverse = ['cycle', 'job'].includes(childNode.type)
   const index = sortedIndexBy(
     parentNode[key],
     childNode,
-    comparator,
+    iteratee,
     { reverse }
   )
   parentNode[key].splice(index, 0, childNode)

tests/unit/components/cylc/common/sort.spec.js

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { DEFAULT_COMPARATOR } from '@/components/cylc/common/sort'
+import { DEFAULT_COMPARATOR, sortedIndexBy } from '@/components/cylc/common/sort'
 
 describe('DEFAULT_COMPARATOR', () => {
   it.each([
@@ -31,3 +31,47 @@ describe('DEFAULT_COMPARATOR', () => {
     expect(DEFAULT_COMPARATOR(a, b)).toEqual(expected)
   })
 })
+
+describe('sortedIndexBy', () => {
+  it.each([
+    [['a', 'c', 'e'], 'a', 0],
+    [['a', 'c', 'e'], 'b', 1],
+    [['a', 'c', 'e'], 'c', 1],
+    [['a', 'c', 'e'], 'd', 2],
+    [['a', 'c', 'e'], 'x', 3],
+    [[], 'x', 0],
+  ])('sortedIndexBy(%o, %o) -> %i', (arr, value, expected) => {
+    expect(sortedIndexBy(arr, value)).toEqual(expected)
+  })
+
+  it('uses the iteratee function to determine the value for comparison', () => {
+    const arr = [{ key: 'a' }, { key: 'c' }, { key: 'e' }]
+    const value = { key: 'b' }
+    const iteratee = (x) => x.key
+    expect(sortedIndexBy(arr, value, iteratee)).toEqual(1)
+  })
+
+  it.each([
+    [[8], 0],
+    [[8, 8], 0],
+    [[8, 8, 8], 3],
+  ])('%#) uses a custom comparator if provided', (value, expected) => {
+    const arr = [[8, 8], [8, 8], [8, 8]]
+    const comparator = (leftObj, leftVal, rightObj, rightVal) => (
+      leftObj.length - rightObj.length
+    )
+    expect(sortedIndexBy(arr, value, (x) => x, { comparator })).toEqual(expected)
+  })
+
+  it('reverses the comparison if reverse option is true', () => {
+    const arr = ['e', 'c', 'a']
+    const value = 'd'
+    expect(sortedIndexBy(arr, value, (x) => x, { reverse: true })).toEqual(1)
+  })
+
+  it('handles numeric collation correctly by default', () => {
+    const arr = ['1', '2']
+    const value = '10'
+    expect(sortedIndexBy(arr, value)).toEqual(2)
+  })
+})