allwpilib icon indicating copy to clipboard operation
allwpilib copied to clipboard

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

Open ori-coval opened this issue 1 year ago • 9 comments

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 avatar May 05 '24 10:05 ori-coval

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

ori-coval avatar May 05 '24 10:05 ori-coval

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

spacey-sooty avatar May 05 '24 13:05 spacey-sooty

no new allocation just return this

ori-coval avatar May 05 '24 13:05 ori-coval

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
  );

SamCarlberg avatar May 05 '24 13:05 SamCarlberg

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?

ori-coval avatar May 05 '24 14:05 ori-coval

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.

PeterJohnson avatar May 05 '24 14:05 PeterJohnson

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);}

ori-coval avatar May 05 '24 15:05 ori-coval

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

KangarooKoala avatar May 05 '24 23:05 KangarooKoala

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.

calcmogul avatar May 06 '24 17:05 calcmogul