beltoforion / muparserx

A C++ Library for Parsing Expressions with Strings, Complex Numbers, Vectors, Matrices and more.
http://beltoforion.de/en/muparserx
BSD 2-Clause "Simplified" License
137 stars 60 forks source link

Github Action workflow changes to run examples (#2) #120

Closed nalinigans closed 1 year ago

nalinigans commented 1 year ago

Note: The builds pass for Windows/Linux, but fail on MacOS because of Issue #116. Not sure if using regex is OK, but please see https://github.com/nalinigans/muparserx/blob/46d69f91cc15a3b3d52556d560a348ce79721117/parser/mpValReader.cpp#L70 for a possible fix to Issue #116.

beltoforion commented 1 year ago

As for the fix. The double value reader should not create complex numbers that just feels wrong. The regex may make this quite slow too. For me it feels like if this is how the solution looks like i'd rather keep the problem. Not sure what to do about this one yet.

nalinigans commented 1 year ago

Agreed that performance will be an issue with regex.

The double value reader should not create complex numbers that just feels wrong

I am probably missing something, are we not doing just that here?

btw experimented using std::atof instead of regular expressions and it seems to work. Would that be a possible solution? See here.

beltoforion commented 1 year ago

Ouch... I forgot about that. In my defense I wrote this code probably more than 6 years ago. However past me was already awesome so I redact my statement from two days ago and would now like to state that there is nothing wrong with this function returning a complex number. My best guess is I never liked it but figured it is a necessary evil because any alternative would be way more difficult.

I may have found something better than atof to fix #116. It seems to work. I have updated the archive accordingly.