Resolve ambiguity between single argument and varargs by replacing varargs to Collection<>

[HeartSaVioR]

Relevant issue: #566

Due to ambiguity issue Scala users should use workaround like...

val stringArray = Array[String]("mystring")
j.del(stringArray:_*)

In order to avoid this issue, as I stated from #566, we should define two methods as...

del(String key); del(String firstKey, String secondKey, String... remainKeys);

which I really don't like it.

Spring Data Redis uses Collection<> to avoid ambiguity. Some of the method definitions are...

https://github.com/spring-projects/spring-data-redis/blob/master/src/main/java/org/springframework/data/redis/core/ScanOptions.java https://github.com/spring-projects/spring-data-redis/blob/master/src/main/java/org/springframework/data/redis/core/ZSetOperations.java

We should use Collection<> at least ambiguity methods, but it hurts consistency. So I suggest we replace all methods using varargs to Collection<> to make consistency.

It changes Jedis APIs completely so this should be applied to only Jedis 3.0.0.

If we really want to apply workaround like k1, k2, keys I'm not sure that it breaks backward compatibility. It anyway could break binary backward compatibility.

[mp911de]

This question kept me busy for a while. I identified about 50 methods which accept varargs and are candidates for some sort of Collection API interface (e.g. Iterable<T>). Varargs is syntax sugar as the compiler creates the bytecode for an array. Using the Collection API requires the user to create always a Set or a List if the method accepts only a Collection interface. Working in parallel, providing varargs and Collection API interfaces together isn't a good idea. This pattern leads to more confusion and a higher risk of bugs as in certain cases the compiler just accepts arguments but does not call the method you'd expect at first sight.

public class MyTest {

    void del(Object... objects) {
        System.out.println("array");
    }

    void del(List<Object> objects) {
        System.out.println("iterable");
    }

    public static void main(String[] args) {

        MyTest mytest = new MyTest();
        mytest.del(Arrays.asList("a"));
    }
}

Output: array

public class MyTest {

    void del(String... objects) {
        System.out.println("array");
    }

    void del(List<String> objects) {
        System.out.println("iterable");
    }

    public static void main(String[] args) {

        MyTest mytest = new MyTest();
        mytest.del(Arrays.asList("a"));
    }
}

Output: iterable

Both, varargs and Collections have their benefits. It's way more convenient for Users dealing with collections of keys/args to pass in their collection but harder to convert a collection to an array. Varargs benefit from syntax sugar. From my reception, varargs work for most Users when using low-level driver libraries. Users of Spring expect a certain consistency in the API and a convenient usage. So the only other (breaking) alternative is having two methods, one with a single argument and the other (overload) accepting a collection interface.

class Jedis {
    … 

   public long del(String key) {…};

   public long del(Iterable<String> keys) {…};

    …
}

(Not saying this is the way to go)

[marcosnils]

@mp911de basically drop support for varargs in replace of Iterable for consistent interfaces. I believe this is a nice proposal and I'd totally like to standardize interfaces for 3.0.

This will also have the benefits of the problems @HeartSaVioR arose when using Scala.

[HeartSaVioR]

Yeah @mp911de thanks for sharing detailed analysis! I also don't think we should maintain both varargs and Collection, and seems like we're good to drop varargs. One question: what do you think about Collection<> vs Iterable<>? Just like to check since SDR uses Collection<> while you're addressing Iterable<>.

@marcosnils Thanks for following up. :)

[mp911de]

Iterable is the lowest possible interface from the Collections API. From a user perspective, it is very convenient invoking methods accepting Iterable. Some libraries provide collection-like types implementing Iterable but not Collection.

I see it also in prospect of the Java 8 Stream-API. A Stream does not implement Iterable because it allows traversing only once yet it gives users access to Iterator. By having a Stream to Iterable utility and the client being aware of the traversing only once nature it would be a nice side-effect of passing a transformed stream into the API.

Various Spring modules offer a broad variety of methods with different signatures (SimpleJpaRepository, BoundGeoOperations). I assume it depends on a lot of factors (evolution over time, author's preferences) why particular methods accept Collection and others Iterable.

[vy]

I am working on a throttler for Log4j 2 Redis Appender. While calling Jedis#rpush(String key, String... strings), I need to provide an array as a second parameter. For that purpose I'm re-using a pre-allocated String[] to avoid allocation at every call. Sometimes even when there are not enough messages to fill the buffer, I still want to push the messages. But since Java does not support shared slicing of arrays, I either need to create a new array and copy the contents from the bigger buffer to here, or just send them one by one each time. Accepting an Iterable or Collection in rpush() would really ease my work.

[github-actions[bot]]

This issue is marked stale. It will be closed in 30 days if it is not updated.