-
Notifications
You must be signed in to change notification settings - Fork 606
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
[wpimath] Add InterpolatingDoubleTreeMap.of() #6635
Conversation
Looks good! I just have one question: Should the method be named |
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.
Yea, it should be called ofEntries().
wpimath/src/main/java/edu/wpi/first/math/interpolation/InterpolatingDoubleTreeMap.java
Outdated
Show resolved
Hide resolved
Co-authored-by: David Vo <auscompgeek@users.noreply.github.com>
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.
Looks good to me, for what it's worth! Just going to note these down for posterity:
- Adding a version for
InterpolatingTreeMap
doesn't make much sense since you'd also need arguments for the interpolator and inverse interpolator, plus that method would also be accessible fromInterpolatingDoubleTreeMap
, which would be confusing - The returned map is mutable, unlike
java.util.Map.ofEntries()
, which returns an immutable map, but that's an acceptable difference. (Maybe even a desirable one)
Co-authored-by: David Vo <auscompgeek@users.noreply.github.com>
Resolves #6586