On Sat, 21 Jan 2023 at 20:00, Ruben Perez <rubenperez038@gmail.com> wrote:
I like how adapt() works. Parsing into compile-time structures is always good. However, as a Redis novice I am, I've had a hard time debugging things when I got a mismatch between the type passed to adapt() and what the command expects. The error happens at parse time, which I think is pretty dangerous, as it's very easy to get things overlooked.
Solving this would require knowing at compile time what data-structure each command returns at compile time, but this is the kind of information that is only available in the documentation of each command (IIRC not even in a form I could use in a generator). As an alternative I could lock-in users on resp3 types: maps, sets, etc. But then how to represent data structures that have no corresponding C++ types. It would also be inefficient as it would require first copying and then converting. In other words, this should be addressed by a higher level library.
As an example of this, the library's own example cpp20_containers, in the hgetall function, uses the type std::map<std::string, std::string> for the hgetall response, which will be okay unless the key is not set, in which case you will get an error. I think this has an enormous potential for causing hidden bugs (and even vulnerabilities). I've also noticed that if you mismatch the sizes of the pipeline request and response, you get an assertion (so UB).
To avoid this you can wrap the response type in std::optional, for example std::optional<std::map<std::string, std::string>> I don't lock users in this behaviour because they might not want it. Perhaps it is desirable to have an error instead of having to check whether the optional has a value. The user must know what he needs. Regards, Marcelo