Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when migrating to 4.0 #148

Closed
Beckdotan opened this issue May 22, 2024 · 11 comments · Fixed by #160
Closed

Error when migrating to 4.0 #148

Beckdotan opened this issue May 22, 2024 · 11 comments · Fixed by #160

Comments

@Beckdotan
Copy link

Hi, Exiting news about the 4.0 version!

I have got this code:
https://dotnetfiddle.net/5iSl8A
That is currently running on my production with Ncalc 3.11 and working beautifully.

I want to migrate to 4.0 to enjoy your hard work for efficiency and the new getParameters function.

But, when doing so, I get few errors and warnings in my code:

but unfortunately when trying to migrate to 4.0 I get few errors:
Compilation error (line 91, col 27): The type or namespace name 'EvaluationVisitor' could not be found (are you missing a using directive or an assembly reference?)

Compilation error (line 93, col 20): The type or namespace name 'EvaluateOptions' could not be found (are you missing a using directive or an assembly reference?)

Compilation error (line 93, col 46): The name 'EvaluateOptions' does not exist in the current context

Can you help me figure it out?

@dkonovalov
Copy link

Read release notes. It was a breaking change, EvaluateOptions has been renamed to ExpressionOptions.
For EvaluationVisitor - I assume you just need correct namespace,
using NCalc.Visitors;

@Beckdotan
Copy link
Author

Beckdotan commented May 22, 2024

@dkonovalov Thanks that worked for this, as you can see in line 95 in my code:
Here but there is many more things going on that are breaking my code that are not in the release note. Here are some I already understood:

  1. The Resultobject in the old EvaluationVisitor used to be declared:
    public object Result { get; protected set; }
    and now in the new code as:
    public object? Result { get; private set; }
    Which caused a problem for me. my solution was to add line 103 and declare it myself.

  2. Needed to change all the rows with the Numbers class to MathHelper class like in row 159 (still need to do it for the rest of the cases.) -> there is still issue that it says it doesn't know the namespce of MathHelprt in the context, why does that happen?
    (maybe because EvaluationVisitor isn't (from problem 3))

  3. it seems like for some reason i couldn't understand it doesn't find the EvaluationVisitoras namespace in line 91

  4. it seems like the code in the EvaluationVisitor and is not longer support my code:
    public MyVisitor(ExpressionOptions options = ExpressionOptions.IgnoreCase, CultureInfo cultureInfo = null) : //V3 change EvaluateOptions -> ExpressionOptions base(options, cultureInfo ?? CultureInfo.CurrentCulture) { CultureInfo = cultureInfo; Parameters = new Dictionary<string, object>(); }
    probebly becuase this function was deleted from the original file:
    public EvaluationVisitor(EvaluateOptions options) : this(options, CultureInfo.CurrentCulture) { }

  5. The function CompareUsingMostPreciseType that i saw that is still public and active is not recognized in scope (maybe because EvaluationVisitor isn't (from problem 3))

Thanks again!

@gumbarros
Copy link
Member

Good morning! I am on the phone and didn't see much. But identified some problems.

  1. Expression.Compile is now LogicalExpressionFactory.Create, the reason is to separe the responsibility of creating a LogicalExpression object from the Expression itself.

  2. You are missing the NCalc.Visitors namespace, the reason of this change was to organize the responsibility of each class and be reflected in folders at the docs site.

At night I will try to help more

@gumbarros
Copy link
Member

gumbarros commented May 22, 2024

Also Numbers became MathHelper

And MathHelper is inside Helpers namespace.

The reason I broke the namespaces is because I and my team normally in other libraries see no problem because you just ctrl+. in the IDE and import the missing namespace.

@Beckdotan
Copy link
Author

Hi thanks (again!!) @gumbarros good morning ;)

Your answers helped a lot!

I still have one problem with the parameters variable.

https://dotnetfiddle.net/yuaMSK

MyVisitor I have this:
public class MyVisitor : EvaluationVisitor
{
public new Dictionary<string, object> Parameters { get; set; } // declare it myself since became privet set in 4.0

But i get the error:
Compilation error (line 103, col 4): 'NCalc.Visitors.EvaluationVisitor.EvaluationVisitor(NCalc.ExpressionOptions, System.Globalization.CultureInfo)' is obsolete: 'Constructors of types with required members are not supported in this version of your compiler.'

now, I cannot update the version wince it will require a big change in the software. and it doesn't happen in the 3.12 version. can you help with that?

@gumbarros
Copy link
Member

gumbarros commented May 23, 2024

Hi thanks (again!!) @gumbarros good morning ;)

Your answers helped a lot!

I still have one problem with the parameters variable.

https://dotnetfiddle.net/yuaMSK

MyVisitor I have this:
public class MyVisitor : EvaluationVisitor
{
public new Dictionary<string, object> Parameters { get; set; } // declare it myself since became privet set in 4.0

But i get the error:
Compilation error (line 103, col 4): 'NCalc.Visitors.EvaluationVisitor.EvaluationVisitor(NCalc.ExpressionOptions, System.Globalization.CultureInfo)' is obsolete: 'Constructors of types with required members are not supported in this version of your compiler.'

now, I cannot update the version wince it will require a big change in the software. and it doesn't happen in the 3.12 version. can you help with that?

Good morning,

Now this is really a bug I will fix, a workaround for you should be installing Polysharp (dependency only on compile time) to allow required members.

@Beckdotan
Copy link
Author

thanks! let me know when its up!

@Beckdotan
Copy link
Author

Beckdotan commented May 26, 2024

@gumbarros Thanks for the work!
Unfortunately, I still get the same error using version 4.0.0 with the code here:
https://dotnetfiddle.net/yuaMSK

will there be 4.0.1 that will come up with this fix?

@gumbarros
Copy link
Member

@gumbarros Thanks for the work!
Unfortunately, I still get the same error using version 4.0.0 with the code here:
https://dotnetfiddle.net/yuaMSK

will there be 4.0.1 that will come up with this fix?

It's fixed at 4.1, will release probably today or monday, just waiting checks at my PRs.

@Beckdotan
Copy link
Author

@gumbarros Amazing!! thanks!!

@Beckdotan
Copy link
Author

@gumbarros Thanks so much! i saw you published the version. now I'm waiting for it to be updated in dotnet fiddle so I can test it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants