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

[wpimath] Add InterpolatingDoubleTreeMap.of() #6635

Merged
merged 3 commits into from
May 20, 2024

Conversation

WispySparks
Copy link
Contributor

Resolves #6586

@WispySparks WispySparks requested a review from a team as a code owner May 18, 2024 02:37
@KangarooKoala
Copy link
Contributor

Looks good! I just have one question: Should the method be named of() or ofEntries()? (java.util.Map uses ofEntries() for the equivalent method)

Copy link
Member

@calcmogul calcmogul left a 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().

Co-authored-by: David Vo <auscompgeek@users.noreply.github.com>
Copy link
Contributor

@KangarooKoala KangarooKoala left a 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:

  1. 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 from InterpolatingDoubleTreeMap, which would be confusing
  2. 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)

@PeterJohnson PeterJohnson merged commit 820f68d into wpilibsuite:main May 20, 2024
25 checks passed
@WispySparks WispySparks deleted the doublemap-of branch May 20, 2024 20:34
chauser pushed a commit to chauser/allwpilib that referenced this pull request May 30, 2024
Co-authored-by: David Vo <auscompgeek@users.noreply.github.com>
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 this pull request may close these issues.

be able to use InterpolatingDoubleTreeMap.put().put()... inline
5 participants