[suggestion] return instead of throw

[lofcz]

Now when invalid token / expression is encountered during evaluation throw statement is used to notify user something went wrong. In scenarios where EE is "proxied" behind some interface (for example a simple web application where I can create and evaluate my scripts) this is suboptimal behavior.

• throw is expensive and shouldn't be used in scenarios where can be triggered often
• it makes hard for users of EE to customize onerror behavior, texts of errors and recover from errors (forces try-catch)
• there is no well defined list of recognized errors, users have to look them up in source code

This suggestion hence proposes that EE.ScriptEvaluate would return ExpressionEvaluatorResult insted of object. ExpressionEvaluatorResult is technically a tuple (could be reduced to a tuple) but for the sake of supporting pre-tuple versions of c# it would be nice to keep this as a class.

public class ExpressionEvaluatorResult {
  public object Result {get; set;} // <-- this is what ScriptEvaluate returns now
  public EvaluationResults State {get; set;} // <-- enum indicating result of evaluation
  public EvaluationResultErrorInfo ErrorInfo {get; set;} // <-- when "ok" this is null
}

Inner members definitions:

public enum EvaluationResults {
   Ok, // <-- first entry indicates the evaluation was successful
   MissingLParen, // <-- following entries all indicate an error
   MissingRParen
   ...
}

public class EvaluationResultErrorInfo {
   public int Line {get; set;} // <-- line in script where error occured
   public int Character {get; set;} // <-- character index in that line
   public string Message {get; set;} // <-- text we now pass to throw
}

Usage would then be:

ExpressionEvaluatorResult result = new ExpressionEvaluator("return 1 + 1;");
if (result.State == EvaluationResults.Ok) {
  // script evaluated successfully
  int ret = (int)result.Result;
}

Please note that all naming used is open for consideration and improvement, it should be as intuitive as possible and this is just from the top of my head.

[lofcz]

@codingseb what do you think about this? Should you give this a go, I can prepare a PR. This is a breaking change but should be an easy one to migrate to.

[codingseb]

if you have A PR you can submit it. But for this issue, I think we need a way to keep compatibility with Exceptions. Could be an option and/or override of methods EE.Evaluate() and EE.ScriptEvaluate(). Also take note that a bunch of tests are relying on this. Some projects too.

We also need to take care of the way it behave when Evaluate() or ScriptEvaluate() method are used in Expressions or scripts with respectively options OptionEvaluateFunctionActive and OptionScriptEvaluateFunctionActive.

[TheoVC]

I have done some tests on the use of try catch and is not more expensive or heavy. by definition an exception should hardly ever happen. only if there was a syntax error ... but that shouldn't happen often