Support ZRANDMEMBER #2076
Support ZRANDMEMBER #2076
Conversation
There was a problem hiding this comment.
I started making suggested changes to plural naming (to match existing methods) for the array cases but after many comments I decided pushing the changes up would be decidedly less annoying :)
This looks great - I tweaked docs and methods, only thing missing I think is testing on the singular methods.
Edit: we should also test the missing key cases (need not be different test methods) to ensure that behaves as expected - thought of this looking at another PR.
Thank you for the help with the docs! |
This also gives clearer errors about what wasn't contained.
NickCraver
left a comment
NickCraver
left a comment
There was a problem hiding this comment.
Looking good, thanks!!
slorello89
left a comment
slorello89
left a comment
There was a problem hiding this comment.
1 comment about the API - looks like it's missing 1 pair of methods.
| /// <param name="flags">The flags to use for this operation.</param> | ||
| /// <returns>The randomly selected element, or <see cref="RedisValue.Null"/> when <paramref name="key"/> does not exist.</returns> | ||
| /// <remarks>https://redis.io/commands/zrandmember</remarks> | ||
| RedisValue SortedSetRandomMember(RedisKey key, CommandFlags flags = CommandFlags.None); |
There was a problem hiding this comment.
Noodling on this, probably want to have a SortedSetRandomMemberWithScore method? Where you just pass in COUNT 1 and the WITHSCORES argument to yield a single SortedSetResult.
There was a problem hiding this comment.
I was thinking: it's rare enough I'm okay leaving it off - if there was ever a complaint, we could add it later.
There was a problem hiding this comment.
@slorello89 If good with the above, I'll go ahead and get this in - thoughts?
| /// <param name="flags">The flags to use for this operation.</param> | ||
| /// <returns>The randomly selected element, or <see cref="RedisValue.Null"/> when <paramref name="key"/> does not exist.</returns> | ||
| /// <remarks>https://redis.io/commands/zrandmember</remarks> | ||
| RedisValue SortedSetRandomMember(RedisKey key, CommandFlags flags = CommandFlags.None); |
3 participants
https://redis-stack.io/commands/zrandmember/
#2055