Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bitfield api #423

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rubicon0501
Copy link

@rubicon0501 rubicon0501 commented Nov 25, 2022

Just add bitfield api

@rubicon0501 rubicon0501 changed the title add bitfield api Add bitfield api Nov 25, 2022
@sewenew
Copy link
Owner

sewenew commented Nov 27, 2022

Thanks for the contribution!

However, it seems that the bitfield interface you proposed, is not more user-friendly than the generic interface: command method:

std::vector<std::string> ops = {"bitfield", key, "SET", "u1", "1", "1","SET", "u1", "3", "0","SET", "u1", "6", "1"};
redis.command<optional<vector<opitonal<long long>>>>(ops.begin(), ops.end());

I checked the doc on redis.io, this command, which has 3 subcommands, is quite complex. It seems that there're 2 solutions:

  1. Encapsulate subcommand into a struct, and pass STL container of this struct as parameter for bitfield:
struct BitFieldSubCommand {
    enum class BitFieldOp {
        GET,
        SET,
        INCR
    };
    std::string encoding;
    long long offset;
    long long value;
    enum class OVERFLOW {
        WRAP,
        SAT,
        FAIL
    };
    OVERFLOW overflow = WRAP;
};

template <typename Input, typename Output>
void bitfield(const StringView &key, Input first, Input last, Output output);

// Usage:
vector<BitFieldSubCommand> subcommands = {BitFieldSubCommand{}, BitFieldSubCommand{}};
redis.bitfield(key, subcommands.begin(), subcommand.end());
  1. Make each subcommand as a container of strings:
template <typename Input, typename Output>
void bitfield(const StringView &key, Input first, Input last, Output output);

// Usage:
vector<vector<string>> subcommands = {{"set", "i4", "10"}, {"get", "i4", "10"}};
redis.bitfield(key, subcommands.begin(), subcommand.end());

However, both solutions are not perfect. The first one is a little complicated, and the GET subcommand doesn't need the value and overflow members. The second is also not more helpful than the generic command interface.

Not quite sure which solution is the best one. Or maybe for complex command, the generic command is the best choice?

B.T.W. you should save the return value of bitfield into a STL container of type optional<long long>, instead of long long. Because if the OVERFLOW type is FAIL, the return value might be nil reply.

Regards

@rubicon0501
Copy link
Author

@sewenew Thank you for reviewing.

@rubicon0501
Copy link
Author

In fact, redis-plus-plus can implement almost all commands through the generic interface: command method, I just want to add the api of bitfield to the project with minimize the use of command, I think it looks more perfect. The first solution of your reply is really too complicated, and the second is better than mine, because it uses type optional<long long>, instead of long long. They all support passing each subcommand as a container of strings. I use this usage because Jedis use this format, so people who have used Jedis before will adapt better.

If you think use command directly is better, we can close the pull request.

@sewenew
Copy link
Owner

sewenew commented Nov 28, 2022

Thanks for the info on Jedis! Let's make this PR pending to see if there're any new feedback from others. Also I'll take a look at other client APIs for these complex command.

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants