PragmaTwice commented on PR #1032:
URL:
https://github.com/apache/incubator-kvrocks/pull/1032#issuecomment-1290298720
> @PragmaTwice After looking through the PR, I was a bit worry that it needs
to take some time for most developers(include myself) to understand how to use
it and how it works. And for Redis command arguments, there're only three
argument types:
>
> * string
> * bool, like NX/EX/PX and so on
> * number(int/float) like TTL and score
>
> So I'm wondering if we can simplify the parser API like below:
>
> ```c++
> while(token = parse.next()) {
> switch tolower(token):
> case "ex":
> status = parser.expect<int>(&ttl)
> case "px":
> status = parser.expect<int64_t>(&ttl_ms)
> ...
> }
> ```
>
> So that users can only concern token and what's next is expected.
I think, there are lots of problem we need to handle:
- We cannot always move `next`: for example, to parse `(EX v1) | (PX v2) |
v3`, we need first `peek` the token (`EX` or `PX`), then we can move `next`,
otherwise we may lose `v3`
- We need a method to forward error: this is where the sample code is
idealized, error handling needs to be abstracted
- We need a method to prevent different flags in the same layer: for
example, to parse `[EX a | PX b] | [X | Y]`, we need to reject something like
`EX v PX v`, `X Y` or `EX v X PX v`, and accept `EX v EX v`, `EX v X` or `Y PX
v`.
Simplifying code means doing good abstraction, and of course good
abstraction has a learning cost, but I still feel that the current abstraction
is intuitive:
- `parser.Good()`: to check if there is still element remain to parse
- `parser.EatICaseFlag(str, flag)`: to match a specific flag token, move
next while sucessful. It can be learned from [this
example](https://github.com/apache/incubator-kvrocks/blob/85ae20ddff43bb71e8370b7a2b19c51c746b6871/tests/cppunit/command_parser_test.cc#L26).
- `parser.TakeInt()` or `parser.TakeStr()`: to eat a new integer or string
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
