Merge d9b2662 into c0b45aa

geoblink · Jul 23, 2019 · 588399e · 588399e
2 parents c0b45aa + d9b2662
commit 588399e
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 10 deletions.
diff --git a/src/mergeForEach.ts b/src/mergeForEach.ts
@@ -15,7 +15,7 @@ export type ComparisonResult = number
  * Can be used to implement efficient algorithms which profit from sorted data
  * like `mergeSort` or `mergeJoinWith`.
  *
- * Both collections must be sortable. They will be sorted ascendently using
+ * Both collections must be sortable. They will be sorted ascendantly using
  * value returned by the corresponding iteratee.
  *
  * @param lhs A collection of elements.
@@ -62,13 +62,11 @@ function mergeForEach<
       const lhsValue = getLHSValue(lhsItem) as any
       const rhsValue = getRHSValue(rhsItem) as any
 
-      if (lhsValue < rhsValue) {
-        return SORTING_ORDER.LHS_BEFORE_RHS
-      } else if (lhsValue > rhsValue) {
-        return SORTING_ORDER.LHS_AFTER_RHS
-      } else {
-        return SORTING_ORDER.EQUAL
-      }
+      if (lhsValue === rhsValue) return SORTING_ORDER.EQUAL
+      if (lhsValue < rhsValue) return SORTING_ORDER.LHS_BEFORE_RHS
+      if (lhsValue > rhsValue) return SORTING_ORDER.LHS_AFTER_RHS
+
+      throw new Error(`[@geoblink/lodash-mixins#mergeForEach] Found non-comparable values: ${lhsValue} on LHS and ${rhsValue} on RHS`)
     }
   }: {
     lhsIteratee?: LHSItemKey | ((item: LHSItem) => any),
@@ -138,7 +136,7 @@ declare module 'lodash' {
      * Can be used to implement efficient algorithms which profit from sorted
      * data like `mergeSort` or `mergeJoinWith`.
      *
-     * Both collections must be sortable. They will be sorted ascendently using
+     * Both collections must be sortable. They will be sorted ascendantly using
      * value returned by the corresponding iteratee.
      *
      * @param lhs A collection of elements.

diff --git a/test/unit/mergeForEach.spec.ts b/test/unit/mergeForEach.spec.ts
@@ -9,6 +9,7 @@ chaiUse(sinonChai)
 describe('mergeForEach', function () {
   const lhsBarcelona = { id: 0, name: 'Barcelona' }
   const lhsMadrid = { id: 1, name: 'Madrid' }
+  const lhsCityWithoutId = { name: 'LHS item without ID' }
 
   const rhsMadrid = { id_city: 1, city_name: 'Madrid' }
   const rhsValencia = { id_city: 2, city_name: 'Valencia' }
@@ -84,6 +85,25 @@ describe('mergeForEach', function () {
     })
   })
 
+  describe('When using default comparison function', function () {
+    describe('When some elements are not comparable', function () {
+      it('Should throw an error', function () {
+        expect(function () {
+          const lhs = [lhsMadrid, lhsBarcelona, lhsCityWithoutId] as any
+          const rhs = [rhsValencia, rhsMadrid, rhsCityWithoutId] as any
+
+          mergeForEach(lhs, rhs, {
+            lhsIteratee: 'id',
+            rhsIteratee: 'id_city',
+            leftCallback,
+            innerCallback,
+            rightCallback
+          })
+        }).to.throw
+      })
+    })
+  })
+
   function buildTestCases<
     L extends any,
     R extends any,
@@ -120,6 +140,7 @@ describe('mergeForEach', function () {
         rightCallback
       })
 
+      expect(leftCallback).to.have.been.calledOnce
       expect(leftCallback).to.have.been.calledWithExactly(lhsBarcelona)
     })
 
@@ -143,8 +164,9 @@ describe('mergeForEach', function () {
         rightCallback
       })
 
-      expect(rightCallback).to.have.been.calledWithExactly(rhsValencia)
+      expect(rightCallback).to.have.been.calledTwice
       expect(rightCallback).to.have.been.calledWithExactly(rhsCityWithoutId)
+      expect(rightCallback).to.have.been.calledWithExactly(rhsValencia)
     })
 
     it('Should not call rightCallback if not provided', function () {