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

be able to use InterpolatingDoubleTreeMap.put().put()... inline #6586

Closed
ori-coval opened this issue May 5, 2024 · 9 comments · Fixed by #6635
Closed

be able to use InterpolatingDoubleTreeMap.put().put()... inline #6586

ori-coval opened this issue May 5, 2024 · 9 comments · Fixed by #6635

Comments

@ori-coval
Copy link
Contributor

Is your feature request related to a problem? Please describe.
for me it is a bit annoying not being able to use .put when creating a new tree map and also not being able to use .put() in a constants class.

Describe the solution you'd like
solution: return the tree when using .put() so you can do as the example below and add all of the data inline.
public static final InterpolatingDoubleTreeMap INTERPOLATION_MAP = new InterpolatingDoubleTreeMap().put(2.9, 62.0).put(2.52 , 55.0).put(2.15 , 50.0).put(3.59, 63.0).put(3.04, 56.0);

@ori-coval
Copy link
Contributor Author

if this is decided to be done I have no prob implementing it.

@spacey-sooty
Copy link
Contributor

I think so long as you don't have to allocate a new object each time this is a fine feature to add

@ori-coval
Copy link
Contributor Author

no new allocation just return this

@SamCarlberg
Copy link
Member

An API shape like Java's existing Map.of but accepting a variadic double array would be preferred.

InterpolatingDoubleTreeMap interpolationMap =
  InterpolatingDoubleTreeMap.of(
    2.90, 62,
    2.52, 55,
    2.25, 50,
    3.59, 63,
    3.04, 56
  );

@ori-coval
Copy link
Contributor Author

I can see why some people would prefer a variadic double array but I can say for myself that I prefer chaining the .put() method for a clear separation between each pair.

so how about adding both?

@PeterJohnson
Copy link
Member

PeterJohnson commented May 5, 2024

I assume the put() call was modeled after the Java Map put() function, which returns the previous value, not the map itself. The current code returns void, but it might be confusing to return this when no other Map in Java does that (including TreeMap). https://docs.oracle.com/javase/8/docs/api/java/util/Map.html#put-K-V-

Is there a reason you want to chain here? You can always put code into a static {} block.

@ori-coval
Copy link
Contributor Author

the reason is ease of use and readability.

I prefer the lower example it makes it easier and faster to add multiple values one after the other and keeps a clear separation between the added values, unlike a variadic double array.
a variadic double array would also be good enough for me but I think chaining would be easier, especially for beginners.

    static {
        shooterAngleInterpolation.put(2.9, 62.0);
        shooterAngleInterpolation.put(2.52, 55.0);
        shooterAngleInterpolation.put(2.15, 50.0);
        shooterAngleInterpolation.put(3.59, 63.0);
        shooterAngleInterpolation.put(3.04, 56.0);
    }

        shooterAngleInterpolation.put(2.9, 62.0)
        .put(2.52, 55.0)
        .put(2.15, 50.0)
        .put(3.59, 63.0)
        .put(3.04, 56.0);}

@KangarooKoala
Copy link
Contributor

What if instead of put() it was named addEntry(), withEntry(), or something else which doesn't have an existing meaning/convention like put()?

@calcmogul
Copy link
Member

calcmogul commented May 6, 2024

We could implement this function from the standard library:
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Map.html#ofEntries(java.util.Map.Entry...)

I strongly suggest we don't deviate from stdlib conventions, because it's intended to act like a stdlib type.

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 a pull request may close this issue.

6 participants