Refactor xlookupArraySize function

Author: sequba

src/interpreter/plugin/LookupPlugin.ts

@@ -149,54 +149,37 @@ export class LookupPlugin extends FunctionPlugin implements FunctionPluginTypech
     })
   }
 
-  public xlookupArraySize(ast: ProcedureAst, state: InterpreterState): ArraySize {
-    const lookupRangeValue = ast?.args?.[1] as CellRange
-    const returnRangeValue = ast?.args?.[2] as CellRange
-
-    if (lookupRangeValue == null
-      || lookupRangeValue.start == null
-      || lookupRangeValue.end == null
-      || returnRangeValue == null
-      || returnRangeValue.start == null
-      || returnRangeValue.end == null
-    ) {
-      return ArraySize.error()
-    }
+  public xlookupArraySize(ast: ProcedureAst): ArraySize {
+    const lookupRange = ast?.args?.[1] as CellRange
+    const returnRange  = ast?.args?.[2] as CellRange
+
+    // co tu wpada jesli argumenty to single-cell range? 
 
-    if (!this.areRangesShapeValidForXlookup(lookupRangeValue, returnRangeValue)) {
+    if (lookupRange?.start == null
+      || lookupRange?.end == null
+      || returnRange?.start == null
+      || returnRange?.end == null
+    ) {
       return ArraySize.error()
     }
 
-    const searchWidth = lookupRangeValue.end.col - lookupRangeValue.start.col + 1 
+    const lookupRangeHeight = lookupRange.end.row - lookupRange.start.row + 1
+    const lookupRangeWidth = lookupRange.end.col - lookupRange.start.col + 1
+    const returnRangeHeight = returnRange.end.row - returnRange.start.row + 1
+    const returnRangeWidth = returnRange.end.col - returnRange.start.col + 1
 
-    if (returnRangeValue?.start == null || returnRangeValue?.end == null) {
-      return ArraySize.scalar()
-    }
+    const isVerticalSearch = lookupRangeWidth === 1 && returnRangeHeight === lookupRangeHeight
+    const isHorizontalSearch = lookupRangeHeight === 1 && returnRangeWidth === lookupRangeWidth
 
-    if (searchWidth === 1) {
-      // column search
-      const outputWidth = returnRangeValue.end.col - returnRangeValue.start.col + 1 
-      return new ArraySize(outputWidth, 1)
-    } else {
-      // row search
-      const outputHeight = returnRangeValue.end.row - returnRangeValue.start.row + 1 
-      return new ArraySize(1, outputHeight)
+    if (!isVerticalSearch && !isHorizontalSearch) {
+      return ArraySize.error()
     }
-  }
-
-  private areRangesShapeValidForXlookup(lookupRange: CellRange, returnRange: CellRange): boolean {
-    const isVerticalSearch = lookupRange.start.col === lookupRange.end.col && lookupRange.start.row <= lookupRange.end.row
-    const isHorizontalSearch = lookupRange.start.row === lookupRange.end.row && lookupRange.start.col <= lookupRange.end.col
 
     if (isVerticalSearch) {
-      return lookupRange.end.row - lookupRange.start.row === returnRange.end.row - returnRange.start.row
+      return new ArraySize(returnRangeWidth, 1)
     }
 
-    if (isHorizontalSearch) {
-      return lookupRange.end.col - lookupRange.start.col === returnRange.end.col - returnRange.start.col
-    }
-
-    return false
+    return new ArraySize(1, returnRangeHeight)
   }
 
   public match(ast: ProcedureAst, state: InterpreterState): InterpreterValue {
@@ -295,7 +278,6 @@ export class LookupPlugin extends FunctionPlugin implements FunctionPluginTypech
     return SimpleRangeValue.onlyValues(returnValues)
   }
 
-
   private doMatch(key: RawNoErrorScalarValue, rangeValue: SimpleRangeValue, type: number): InternalScalarValue {
     if (![-1, 0, 1].includes(type)) {
       return new CellError(ErrorType.VALUE, ErrorMessage.BadMode)

test/interpreter/function-xlookup.spec.ts

@@ -139,8 +139,8 @@ describe('Function XLOOKUP', () => {
 
       expect(engine.getCellValue(adr('A1'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.ValueNotFound))
       expect(engine.getCellValue(adr('A2'))).toEqualError(detailedError(ErrorType.NA, ErrorMessage.ValueNotFound))
-      expect(engine.getCellValue(adr('A3'))).toEqual("not found")
-      expect(engine.getCellValue(adr('A4'))).toEqual("not found")
+      expect(engine.getCellValue(adr('A3'))).toEqual('not found')
+      expect(engine.getCellValue(adr('A4'))).toEqual('not found')
     })
 
     it('works when returnArray is shifted (verical search)', () => {
@@ -172,24 +172,45 @@ describe('Function XLOOKUP', () => {
       expect(engine.getCellValue(adr('A1'))).toEqual('b')
     })
 
-    it('works when lookupArray is a single cell', () => {
-      const engine = HyperFormula.buildFromArray([
-        ['=XLOOKUP(1, B1:B1, C1:C1)', 1, 'a', 'horizontal'], // lookupArray: single cell, returnArray: single cell
-        ['=XLOOKUP(1, B1, C1:C1)', '', 'vertical'], // lookupArray: single cell, returnArray: single cell
-        ['=XLOOKUP(1, B1:B1, C1:C2)'], // lookupArray: single cell, returnArray: vertical range
-        [],
-        ['=XLOOKUP(1, B1, C1:C2)'], // lookupArray: single cell, returnArray: vertical range
-        [],
-        ['=XLOOKUP(1, B1:B1, C1:D1)'], // lookupArray: single cell, returnArray: horizontal range
-        ['=XLOOKUP(1, B1, C1:D1)'], // lookupArray: single cell, returnArray: horizontal range
-      ])
-
-      expect(engine.getCellValue(adr('A1'))).toEqual('a')
-      expect(engine.getCellValue(adr('A2'))).toEqual('a')
-      expect(engine.getRangeValues(AbsoluteCellRange.spanFrom(adr('A3'), 1, 2))).toEqual([['a'], ['vertical']])
-      expect(engine.getRangeValues(AbsoluteCellRange.spanFrom(adr('A5'), 1, 2))).toEqual([['a'], ['vertical']])
-      expect(engine.getRangeValues(AbsoluteCellRange.spanFrom(adr('A7'), 2, 1))).toEqual([['a', 'horizontal']])
-      expect(engine.getRangeValues(AbsoluteCellRange.spanFrom(adr('A8'), 2, 1))).toEqual([['a', 'horizontal']])
+    describe('when lookupArray is a single cell', () => {
+      it('works when returnArray is also a single cell', () => {
+        const engine = HyperFormula.buildFromArray([
+          ['=XLOOKUP(1, B1:B1, C1:C1)', 1, 'a'],
+          ['=XLOOKUP(1, B1, C1:C1)'],
+        ])
+
+        expect(engine.getCellValue(adr('A1'))).toEqual('a')
+        expect(engine.getCellValue(adr('A2'))).toEqual('a')
+      })
+
+      it('works when returnArray is a vertical range', () => {
+        const engine = HyperFormula.buildFromArray([
+          ['=XLOOKUP(1, B1:B1, A5:A6)', 1],
+          [],
+          ['=XLOOKUP(1, B1, A5:A6)'],
+          [],
+          ['b'],
+          ['c']
+        ])
+
+        expect(engine.getCellValue(adr('A1'))).toEqual('b')
+        expect(engine.getCellValue(adr('A2'))).toEqual('c')
+        expect(engine.getCellValue(adr('A3'))).toEqual('b')
+        expect(engine.getCellValue(adr('A4'))).toEqual('c')
+      })
+
+      it('works when returnArray is a horizontal range', () => {
+        const engine = HyperFormula.buildFromArray([
+          [1, 'b', 'c'],
+          ['=XLOOKUP(1, A1:A1, B1:C1)'],
+          ['=XLOOKUP(1, A1, B1:C1)'],
+        ])
+
+        expect(engine.getCellValue(adr('A2'))).toEqual('b')
+        expect(engine.getCellValue(adr('B2'))).toEqual('c')
+        expect(engine.getCellValue(adr('A3'))).toEqual('b')
+        expect(engine.getCellValue(adr('B3'))).toEqual('c')
+      })
     })
 
     it('finds an empty cell', () => {
@@ -294,5 +315,7 @@ describe('Function XLOOKUP', () => {
 })
 
 // TODO:
-// - single cell
-// - modes
+// - debugger
+// - review arraysize function
+// - fix single cell
+// - implementmodes