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

Change ToPrimitive() (to_primitive()) hint to be an enum, instead of string #466

Closed
HalidOdat opened this issue Jun 8, 2020 · 0 comments · Fixed by #470
Closed

Change ToPrimitive() (to_primitive()) hint to be an enum, instead of string #466

HalidOdat opened this issue Jun 8, 2020 · 0 comments · Fixed by #470
Labels
E-Easy Easy execution Issues or PRs related to code execution good first issue Good for newcomers performance Performance related changes and issues technical debt
Milestone

Comments

@HalidOdat
Copy link
Member

What's wrong with the current implementation of to_primitive?
The problem is that we use a string (&str) as the hint, so when we use to_primitive we always have to do a string comparison, which is not efficient.

To fix this we need to create a new enum PreferredType which would contain String, Number and Default field and use it to compare it instead of &str

More information:

@HalidOdat HalidOdat added good first issue Good for newcomers performance Performance related changes and issues E-Easy Easy technical debt execution Issues or PRs related to code execution labels Jun 8, 2020
@Razican Razican added this to the v0.9.0 milestone Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-Easy Easy execution Issues or PRs related to code execution good first issue Good for newcomers performance Performance related changes and issues technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants