redis-rb

redis-rb copied to clipboard

I'd like to suggest removing the Boolify from non-variadic ZADD and SADD invocations (also SREM / ZREM). I understand and agree with the boolean casting for most methods that do so, but the Redis documentation is unambiguous about the meaning of these return values: "the number of elements that were added to the set, not including all the elements already present into the set." (https://redis.io/commands/sadd). For someone who is familiar with Redis, returning booleans from SADD and ZADD will be confusing at first, and I don't see the benefit.

There are other considerations as well. As an example, the current implementation creates an inconsistency with variadic invocations with only one value (or pair in the case of ZSET).

redis.sadd("myset", "member1")
# => true

redis.sadd("myset", ["member2"])
# => 1

These calls are equivalent from the perspective of Redis, but give different return values with redis-rb.

As a user, I would expect redis-rb to produce the closest representation to the actual return value in Redis, except for cases where a boolean is obviously intended (e.g. EXISTS) or explicitly stated in the Redis documentation (as in something like HSET). There may be other paradigms where Set#add returns a boolean, but (as far as I know) that is not currently the case in Ruby or Redis.

If the beneficent maintainers agree, I've created a pull request here: #819.

As explained in #819 this cannot be changed for backward compatibility reasons.

It started returning a boolean a long time ago when Redis would only take a single member, so yeah, once multi-members support was added, the API started to get a bit awkward.

I'll leave this issue open as it could be considered for a future breaking change release, but It's not expected to happen any time soon.

I'm about to release a 5.0, so that would be the occasion to change this.

I'll sleep on it though.