-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add multi-expression plotting with color selection etc #11
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
feat: add multi-expression plotting with color selection etc #11
Conversation
|
LGTM, I'll review and merge this by this evening |
| #include "exprtk.hpp" | ||
| #include "funcs.hpp" | ||
|
|
||
| using namespace std; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the use of using namespace std. It is bad practice and it will create trouble at some point, if not now. See https://stackoverflow.com/questions/1452721/whats-the-problem-with-using-namespace-std for further reading.
| string s(e.expr); | ||
| bool ok = false; | ||
|
|
||
| if (!s.empty() && s.front() == '(' && s.back() == ')') { | ||
| string inner = s.substr(1, s.size() - 2); | ||
| int d = 0; | ||
| size_t pos = string::npos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also refrain from shortening variables for no reason. It makes the code unreadable. s should be renamed to func_str, d should be renamed to depth, and ok should be renamed to plotted.
| exprtk::symbol_table<double> st; | ||
| st.add_constants(); | ||
| addConstants(st); | ||
| st.add_variable("t", t); | ||
| exprtk::expression<double> ef, eg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, rename st to symbolTable and ef, eg to expr_fx, expr_gx
| exprtk::symbol_table<double> st; | ||
| st.add_constants(); | ||
| addConstants(st); | ||
| st.add_variable("x", x); | ||
| exprtk::expression<double> expr; | ||
| expr.register_symbol_table(st); | ||
| exprtk::parser<double> p; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing, rename the variables here as well
|
@saumya1317 Important: I would suggest you to wait until I merge both of the other PRs. You will have to resolve merge conflicts and thus I would suggest you to only start working on my requested changes once I merge the other 2 open PRs. I'll ping you here once I do that. |
|
ok |
This pull request adds multi-expression plotting capabilities to the graphing application, similar to Desmos. Users can now plot multiple mathematical expressions simultaneously, each with its own customizable color. The feature includes dynamic expression management (add/remove), individual enable/disable toggles, and support for both regular functions and parametric curves.
Semver Changes
Issues
Closes #8 (Add support for plotting multiple expressions)
Checklist