zrevrangebyscore parameters are incorrect

[alexanderscott]

zrevrangebyscore and zrevrangebyscoreWithScores should take the max first and min second as parameters (docs). These are reversed in the Sorted Set commands.

[etaty]

nah just use named parameters max = ... then min = ... haha

Yeah now the question is, if we reverse the order but someone already use the functions?

[alexanderscott]

Maybe I'm misunderstanding, but using named params would still yield the same result since they are passed in the same order. For ex/ zrevrangebyscore(key = "myKey", min = Limit(0.00), max = Limit(10.00)) will not return members with scores between 0 and 10 like it should.

Yeah... swapping the names of the params so they are accurate (e.g. (key: String, max: Limit, min: Limit, limit: Option[(Long, Long)] = None)) would cause an issue for anyone who already may be using named params. Though in that scenario they would have to be purposefully naming them incorrectly to compensate.

What if it did a comparison of the min and max, and applied the lower value to the min and higher value to the max? That way it wouldn't matter the name or order, and would support legacy use-cases. Can you think of any examples where one would pass a higher value for the min?

[etaty]

Oh I just have realized i have also inverted the parameters in the request https://github.com/etaty/rediscala/blob/master/src/main/scala/redis/api/SortedSets.scala#L83

Your solution could work, I don't see a case where someone would want an empty list. The redis api reversing the order of the parameters with ZRANGEBYSCORE is not really friendly.

127.0.0.1:6379> ZADD myzset 1 "one"
(integer) 1
127.0.0.1:6379> ZADD myzset 2 "two"
(integer) 1
127.0.0.1:6379> ZADD myzset 3 "three"
(integer) 1
127.0.0.1:6379> ZREVRANGEBYSCORE myzset +inf -inf
1) "three"
2) "two"
3) "one"
127.0.0.1:6379> ZREVRANGEBYSCORE myzset -inf +inf
(empty list or set)
127.0.0.1:6379> ZRANGEBYSCORE myzset -inf +inf
1) "one"
2) "two"
3) "three"
127.0.0.1:6379> ZRANGEBYSCORE myzset +inf -inf
(empty list or set)

[alexanderscott]

Yeah, it's a pain that the Redis API reverses them like that. Created pull #99 with the workaround I mentioned above.