Skip to content

Commit

Permalink
fix(formula): cache false as blank array value
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dushusir
Dushusir committed Apr 8, 2024
1 parent 8b06b4f commit 42f7f3b
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 9 deletions.
82 changes: 81 additions & 1 deletion packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@

import { describe, expect, it } from 'vitest';

import type { Nullable } from '@univerjs/core';
import { ArrayValueObject, transformToValueObject, ValueObjectFactory } from '../array-value-object';
import type { BooleanValueObject, NumberValueObject } from '../primitive-object';
import { BooleanValueObject, NumberValueObject } from '../primitive-object';
import type { BaseValueObject } from '../base-value-object';
import { ErrorValueObject } from '../base-value-object';
import { ErrorType } from '../../../basics/error-type';

describe('arrayValueObject test', () => {
const originArrayValueObject = ArrayValueObject.create({
Expand Down Expand Up @@ -397,4 +401,80 @@ describe('arrayValueObject test', () => {
expect(errorValueObject.isError()).toBeTruthy();
});
});

describe('Test DefaultValue', () => {
it('After set defaultValue, use ', () => {
const calculateValueList: Nullable<BaseValueObject>[][] = [[]];
calculateValueList[0][1] = NumberValueObject.create(2);

const arrayValueObject = ArrayValueObject.create({
calculateValueList,
rowCount: 2,
columnCount: 2,
unitId: '',
sheetId: '',
row: 0,
column: 0,
});

arrayValueObject.setDefaultValue(BooleanValueObject.create(false));

// getFirstCell
expect(arrayValueObject.getFirstCell().getValue()).toStrictEqual(false);

// getLastCell
expect(arrayValueObject.getLastCell().getValue()).toStrictEqual(false);

// iterator
arrayValueObject.iterator(iterator);

// iteratorReverse
arrayValueObject.iteratorReverse(iterator);

arrayValueObject.mapValue(mapValue);

function iterator(valueObject: Nullable<BaseValueObject>, row: number, column: number) {
if (!valueObject) {
return;
}

const value = valueObject.getValue();

if (row === 0 && column === 0) {
expect(value).toStrictEqual(false);
} else if (row === 0 && column === 1) {
expect(value).toStrictEqual(2);
} else if (row === 1 && column === 0) {
expect(value).toStrictEqual(false);
} else if (row === 1 && column === 1) {
expect(value).toStrictEqual(false);
}
}

function mapValue(valueObject: Nullable<BaseValueObject>, row: number, column: number) {
if (!valueObject) {
return ErrorValueObject.create(ErrorType.NA);
}

iterator(valueObject, row, column);

return valueObject;
}

// flatten
const flatten = arrayValueObject.flatten();
expect(flatten.getFirstCell().getValue()).toStrictEqual(false);

// slice
const firstColumn = arrayValueObject.slice([0, 1], [0, 1]);
if (!firstColumn) {
throw new Error('firstColumn is null');
}
expect(firstColumn.getFirstCell().getValue()).toStrictEqual(false);

// transpose
const transpose = arrayValueObject.transpose();
expect(transpose.getFirstCell().getValue()).toStrictEqual(false);
});
});
});
37 changes: 29 additions & 8 deletions packages/engine-formula/src/engine/value-object/array-value-object.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ export class ArrayValueObject extends BaseValueObject {

private _flattenCache: Nullable<ArrayValueObject>;

/**
* The default value of the array, null values in comparison results support setting to false
*/
private _defaultValue: Nullable<BaseValueObject> = null;

private _flattenPosition: Nullable<{
stringArray: BaseValueObject[];
stringPosition: number[];
Expand Down Expand Up @@ -212,6 +217,10 @@ export class ArrayValueObject extends BaseValueObject {
return true;
}

setDefaultValue(value: Nullable<BaseValueObject>) {
this._defaultValue = value;
}

get(row: number, column: number) {
const rowValues = this._values[row];
if (rowValues == null) {
Expand Down Expand Up @@ -271,7 +280,7 @@ export class ArrayValueObject extends BaseValueObject {

for (let r = startRow; r <= endRow; r++) {
for (let c = startColumn; c <= endColumn; c++) {
if (callback(valueList[r]?.[c], r, c) === false) {
if (callback(valueList[r]?.[c] || this._defaultValue, r, c) === false) {
return;
}
}
Expand All @@ -287,7 +296,7 @@ export class ArrayValueObject extends BaseValueObject {

for (let r = endRow; r >= startRow; r--) {
for (let c = endColumn; c >= startColumn; c--) {
if (callback(valueList[r][c], r, c) === false) {
if (callback(valueList[r]?.[c] || this._defaultValue, r, c) === false) {
return;
}
}
Expand Down Expand Up @@ -330,12 +339,12 @@ export class ArrayValueObject extends BaseValueObject {

getFirstCell() {
const { startRow, startColumn } = this.getRangePosition();
return this.get(startRow, startColumn) || NullValueObject.create();
return this.get(startRow, startColumn) || this._defaultValue || NullValueObject.create();
}

getLastCell() {
const { endRow, endColumn } = this.getRangePosition();
return this.get(endRow, endColumn) || NullValueObject.create();
return this.get(endRow, endColumn) || this._defaultValue || NullValueObject.create();
}

/**
Expand Down Expand Up @@ -395,6 +404,8 @@ export class ArrayValueObject extends BaseValueObject {

const arrayV = this._createNewArray(newValue, 1, newValue[0].length);

arrayV.setDefaultValue(this._defaultValue);

this._flattenCache = arrayV;

return arrayV;
Expand Down Expand Up @@ -504,7 +515,7 @@ export class ArrayValueObject extends BaseValueObject {
return;
};

let cell = array[r][c];
let cell = array[r][c] || this._defaultValue;

if (cell == null) {
cell = NullValueObject.create();
Expand Down Expand Up @@ -533,6 +544,9 @@ export class ArrayValueObject extends BaseValueObject {

const newResultArray = this._createNewArray(result, result.length, result[0].length, startRow, startColumn);

// Synchronize defaultValue
newResultArray.setDefaultValue(this._defaultValue);

this._sliceCache.set(cacheKey, newResultArray);

return newResultArray;
Expand Down Expand Up @@ -560,7 +574,10 @@ export class ArrayValueObject extends BaseValueObject {
const rowCount = this._rowCount;
const columnCount = this._columnCount;

return this._createNewArray(transposeArray, columnCount, rowCount);
const newArray = this._createNewArray(transposeArray, columnCount, rowCount);

newArray.setDefaultValue(this._defaultValue);
return newArray;
}

/**
Expand Down Expand Up @@ -934,7 +951,7 @@ export class ArrayValueObject extends BaseValueObject {
if (row == null) {
rowList[c] = ErrorValueObject.create(ErrorType.VALUE);
} else {
const currentValue = row[c];
const currentValue = row[c] || this._defaultValue;

if (currentValue) {
rowList[c] = callbackFn(currentValue, r, c);
Expand Down Expand Up @@ -1372,7 +1389,11 @@ export class ArrayValueObject extends BaseValueObject {
this._batchOperatorValue(value, c, result, batchOperatorType, operator);
}

return this._createNewArray(result, rowCount, columnCount);
const newArray = this._createNewArray(result, rowCount, columnCount);

// Mark empty values in the array as false
newArray.setDefaultValue(BooleanValueObject.create(false));
return newArray;
}

private _batchOperatorValue(
Expand Down

0 comments on commit 42f7f3b

Please sign in to comment.