Skip to content

Commit

Permalink
Robmarti12/convert compile to static (#17) (#18)
Browse files Browse the repository at this point in the history
* Robmarti12/convert compile to static (#17)

* Refactor error handling in Expression class

Removed the ErrorListener instances in the Expression class and simplified the error handling process. These changes reduced redundancy and improved the readability of the code. Now, instead of using two separate ErrorListener instances to catch lexer and parser errors, we just throw an error in case parsing fails. Errors are now stored directly in an array in the Expression instance, simplifying access to them.

* Refactor tests for better error handling and clean up

Refactored Expression.test.ts and JsonSerialize.test.ts to utilize HasErrors() function for clearer, more accurate error handling. Additionally, cleaned up unnecessary variable declarations in JsonSerialize.test.ts for better readability. These changes improve test robustness and clarity in code.

---------
Co-authored-by: robmarti12 <robin.martin@cmcitymedia.de>
Co-authored-by: Thomas Hambach <thambach@gmail.com>
  • Loading branch information
ThomasHambach committed Aug 19, 2023
1 parent d8d0361 commit 163a429
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 29 deletions.
11 changes: 6 additions & 5 deletions __tests__/Expression.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import 'jest-expect-message';
import { Expression } from '../src/NCalc/Expression';
import { FunctionArgs } from '../src/NCalc/FunctionArgs';
import { ParameterArgs } from '../src/NCalc/ParameterArgs';
import {Expression} from '../src/NCalc/Expression';
import {FunctionArgs} from '../src/NCalc/FunctionArgs';
import {ParameterArgs} from '../src/NCalc/ParameterArgs';
import {
BinaryExpression,
BinaryExpressionType,
Identifier,
NCalcFunction,
ValueExpression
} from '../src/NCalc/Domain/index';
import { EvaluateOptions } from '../src/NCalc/EvaluationOptions';
import {EvaluateOptions} from '../src/NCalc/EvaluationOptions';

describe('Expressions', () => {
test('ShouldCache', () => {
const expression = new Expression('1 + 2');
expect(Expression.CachedExpressions['1 + 2']).toBe(undefined);
expression.Evaluate();
Expression.Compile('1 + 2', false);
expect(Expression.CachedExpressions).toHaveProperty('1 + 2');
});

Expand Down Expand Up @@ -364,6 +364,7 @@ describe('Expressions', () => {
let e = new Expression('a + b * (');
expect(e.errors.length).toBe(0);
expect(e.HasErrors()).toBeTruthy();
expect(e.HasErrors()).toBeTruthy();
expect(e.errors.length).toBe(1);

e = new Expression('* b ');
Expand Down
8 changes: 3 additions & 5 deletions __tests__/JsonSerialize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import { EvaluationVisitor } from '../src/NCalc/Domain';

describe('Expressions', () => {
test('ShouldSerializeSimple', () => {
const expression = new Expression('1 + 1');
const parsed = expression.Compile('1 = 1 AND 5 = 5 AND (1 = 1', true);
const parsed = Expression.Compile('1 = 1 AND 5 = 5 AND (1 = 1)', true);
const serialized = JSON.stringify(parsed);
const parsedTree = EvaluationVisitor.FromJson(JSON.parse(serialized));
const exp = new Expression(parsedTree);
Expand All @@ -14,9 +13,8 @@ describe('Expressions', () => {
});

test('ShouldSerializeComplex', () => {
const expression = new Expression('1 + 1');
const expressionString = '(((bfid % 389) >=0 AND (bfid % 389) <= 340) AND bfid != 1876702 AND bfid != 1806269 AND bfid != 2031020 AND bfid != 2039347 AND bfid != 1904629 AND bfid != 1864103 AND bfid != 1935667 AND bfid != 1806056 AND bfid != 1882735 AND bfid != 2094459 AND bfid != 1866312 AND bfid != 93833 AND bfid != 95678 AND bfid != 2077999 AND bfid != 2136701 AND bfid != 1882689 AND bfid != 1938176 AND bfid != 36100 AND bfid != 2075982 AND bfid != 2098624 AND bfid != 2105977 AND bfid != 2249726) AND time >= 1678176000000 AND time <= 1683532800000';
const parsed = expression.Compile(expressionString, true);
const parsed = Expression.Compile(expressionString, true);
const serialized = JSON.stringify(parsed);
const parsedTree = EvaluationVisitor.FromJson(JSON.parse(serialized));
const exp = new Expression(parsedTree);
Expand All @@ -25,4 +23,4 @@ describe('Expressions', () => {
expect(exp.Evaluate()).toBe(false);
});

});
});
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ncalcjs",
"version": "2.1.0",
"version": "2.1.1",
"description": "Typescript/Javascript implementation of NCalc",
"main": "dist/ncalc.node.js",
"browser": "dist/ncalc.web.js",
Expand Down
32 changes: 15 additions & 17 deletions src/NCalc/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import { ErrorListener } from './ErrorListener';

export class Expression {

private lexerErrors: ErrorListener;

private parserErrors: ErrorListener;

public Options: EvaluateOptions = EvaluateOptions.None;

/**
Expand Down Expand Up @@ -67,19 +63,16 @@ export class Expression {
if (options) {
this.Options = options;
}

this.lexerErrors = new ErrorListener();
this.parserErrors = new ErrorListener();
}

public get errors() {
return this.lexerErrors.errors.concat(this.parserErrors.errors);
}
public errors: any[] = [];

public Compile(expression: string, nocache: boolean): LogicalExpression {
public static Compile(expression: string, nocache: boolean): LogicalExpression {
let logicalExpression: LogicalExpression | null = null;
const lexerErrors = new ErrorListener();
const parserErrors = new ErrorListener();

if (this.CacheEnabled && !nocache) {
if (Expression._cacheEnabled && !nocache) {
if (Object.prototype.hasOwnProperty.call(Expression._compiledExpression, expression)) {
const wr = Expression._compiledExpression[expression];
const stored = wr.deref();
Expand All @@ -93,16 +86,20 @@ export class Expression {
// Create the lexer
const inputStream = new antlr4.CharStream(expression);
const lexer = new NCalcLexer(inputStream);
lexer.addErrorListener(this.lexerErrors);
lexer.addErrorListener(lexerErrors);

// Create parser
const tokenStream = new antlr4.CommonTokenStream(lexer);
const parser = new NCalcParser(tokenStream);
parser.addErrorListener(this.parserErrors);
parser.addErrorListener(parserErrors);

logicalExpression = (parser as any).GetExpression();

if (this.CacheEnabled && !nocache) {
if(lexerErrors.errors.length > 0 || parserErrors.errors.length > 0) {
throw new Error('Failed to parse expression');
}

if (Expression._cacheEnabled && !nocache) {
Expression._compiledExpression[expression] = new WeakRef(logicalExpression);
}
}
Expand All @@ -117,7 +114,7 @@ export class Expression {
public HasErrors(): boolean {
try {
if (this.ParsedExpression == null) {
this.ParsedExpression = this.Compile(
this.ParsedExpression = Expression.Compile(
this.OriginalExpression,
(this.Options & EvaluateOptions.NoCache) == EvaluateOptions.NoCache
);
Expand All @@ -130,6 +127,7 @@ export class Expression {
// In case HasErrors() is called multiple times for the same expression
return this.ParsedExpression === null || this.ParsedExpression === undefined;
} catch (e) {
this.errors = [e];
return true;
}
}
Expand All @@ -143,7 +141,7 @@ export class Expression {
}

if (this.ParsedExpression == null) {
this.ParsedExpression = this.Compile(
this.ParsedExpression = Expression.Compile(
this.OriginalExpression,
(this.Options & EvaluateOptions.NoCache) == EvaluateOptions.NoCache
);
Expand Down

0 comments on commit 163a429

Please sign in to comment.