Skip to content

Commit

Permalink
fix(formula): vlookup reuse function
Browse files Browse the repository at this point in the history
  • Loading branch information
Dushusir committed Mar 28, 2024
1 parent 850afa2 commit 3ac0139
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 39 deletions.
8 changes: 5 additions & 3 deletions packages/engine-formula/src/engine/utils/array-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,16 @@ export function expandArrayValueObject(rowCount: number, columnCount: number, va
export function createNewArray(
result: BaseValueObject[][],
rowCount: number,
columnCount: number
columnCount: number,
unitId = '',
sheetId = ''
) {
const arrayValueObjectData: IArrayValueObject = {
calculateValueList: result,
rowCount,
columnCount,
unitId: '',
sheetId: '',
unitId,
sheetId,
row: -1,
column: -1,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ describe('Test vlookup', () => {
const resultObject = textFunction.calculate(
StringValueObject.create('lookupValue'),
arrayValueObject1.clone(),
colIndexNumArrayValueObject2.clone(),
ArrayValueObject.create(/*ts*/ `{
3, 4;
}`),
NumberValueObject.create(0)
) as BaseValueObject;
expect(getObjectValue(resultObject)).toStrictEqual(ErrorType.REF);
Expand Down
71 changes: 36 additions & 35 deletions packages/engine-formula/src/functions/lookup/vlookup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
* limitations under the License.
*/

import type { Nullable } from '@univerjs/core';
import { ErrorType } from '../../../basics/error-type';
import { expandArrayValueObject } from '../../../engine/utils/array-object';
import { createNewArray, expandArrayValueObject } from '../../../engine/utils/array-object';
import type { ArrayValueObject } from '../../../engine/value-object/array-value-object';
import type { BaseValueObject } from '../../../engine/value-object/base-value-object';
import { ErrorValueObject } from '../../../engine/value-object/base-value-object';
Expand Down Expand Up @@ -65,40 +66,34 @@ export class Vlookup extends BaseFunction {
return ErrorValueObject.create(ErrorType.VALUE);
}

let errorValue: ErrorValueObject | undefined;
let errorValue: BaseValueObject | undefined;
const result: BaseValueObject[][] = [];

const result = colIndexNum.map((colIndexNumValueObject: BaseValueObject) => {
const colIndexNumValue = this.getIndexNumValue(colIndexNumValueObject);

if (colIndexNumValue instanceof ErrorValueObject) {
errorValue = colIndexNumValue;
return errorValue;
(colIndexNum as ArrayValueObject).iterator((colIndexNumValueObject: Nullable<BaseValueObject>, rowIndex: number, columnIndex: number) => {
if (colIndexNumValueObject === null || colIndexNumValueObject === undefined) {
errorValue = ErrorValueObject.create(ErrorType.VALUE);
return false;
}

const searchArray = (tableArray as ArrayValueObject).slice(undefined, [0, 1]);
const searchObject = this._handleTableArray(lookupValue, tableArray, colIndexNumValueObject, rangeLookupValue);

if (searchArray == null) {
errorValue = ErrorValueObject.create(ErrorType.VALUE);
return errorValue;
if (searchObject.isError()) {
errorValue = searchObject;
return false;
}

const resultArray = (tableArray as ArrayValueObject).slice(undefined, [colIndexNumValue - 1, colIndexNumValue]);

if (resultArray == null) {
errorValue = ErrorValueObject.create(ErrorType.REF);
return errorValue;
if (result[rowIndex] === undefined) {
result[rowIndex] = [];
}

// The error reporting priority of lookupValue in Excel is higher than colIndexNum. It is required to execute the query from the first column first, and then take the value of colIndexNum column from the query result.
// Here we will first throw the colIndexNum error to avoid unnecessary queries and improve performance.
return this._handleSingleObject(lookupValue, searchArray, resultArray, rangeLookupValue);
result[rowIndex][columnIndex] = searchObject;
});

if (errorValue) {
return errorValue;
}

return result;
return createNewArray(result, result.length, result[0].length, this.unitId || '', this.subUnitId || '');
}

// max row length
Expand Down Expand Up @@ -137,26 +132,32 @@ export class Vlookup extends BaseFunction {
return ErrorValueObject.create(ErrorType.VALUE);
}

const colIndexNumValue = this.getIndexNumValue(colIndexNum);
return this._handleTableArray(lookupValue, tableArray, colIndexNum, rangeLookupValue);
});
}

if (colIndexNumValue instanceof ErrorValueObject) {
return colIndexNumValue;
}
private _handleTableArray(lookupValue: BaseValueObject, tableArray: BaseValueObject, colIndexNum: BaseValueObject, rangeLookupValue: number) {
const colIndexNumValue = this.getIndexNumValue(colIndexNum);

const searchArray = (tableArray as ArrayValueObject).slice(undefined, [0, 1]);
if (colIndexNumValue instanceof ErrorValueObject) {
return colIndexNumValue;
}

if (searchArray == null) {
return ErrorValueObject.create(ErrorType.VALUE);
}
const searchArray = (tableArray as ArrayValueObject).slice(undefined, [0, 1]);

const resultArray = (tableArray as ArrayValueObject).slice(undefined, [colIndexNumValue - 1, colIndexNumValue]);
if (searchArray == null) {
return ErrorValueObject.create(ErrorType.VALUE);
}

if (resultArray == null) {
return ErrorValueObject.create(ErrorType.REF);
}
const resultArray = (tableArray as ArrayValueObject).slice(undefined, [colIndexNumValue - 1, colIndexNumValue]);

return this._handleSingleObject(lookupValue, searchArray, resultArray, rangeLookupValue);
});
// The error reporting priority of lookupValue in Excel is higher than colIndexNum. It is required to execute the query from the first column first, and then take the value of colIndexNum column from the query result.
// Here we will first throw the colIndexNum error to avoid unnecessary queries and improve performance.
if (resultArray == null) {
return ErrorValueObject.create(ErrorType.REF);
}

return this._handleSingleObject(lookupValue, searchArray, resultArray, rangeLookupValue);
}

private _handleSingleObject(
Expand Down

0 comments on commit 3ac0139

Please sign in to comment.