DiceDB / dice

DiceDB is hyper-optimized for building and scaling truly real-time applications on modern hardware. It is a drop-in replacement for Redis with support for SQL-based reactivity.
https://dicedb.io/
Other
3.32k stars 420 forks source link

#182: Refactor command handling for maintainability #170

Closed kakdeykaushik closed 1 month ago

kakdeykaushik commented 1 month ago

182

Dice cmds via struct

Summary of change: Designed a struct to implement dice commands

Implementation details: DiceCmdMeta encapsulates information about a dice command. A map map[string]DiceCmdMeta{} stores all the DiceCmdMeta objects and acts as a source of truth for all the commands.

Redis reference:

Screenshot 2024-07-19 at 16 40 08

Note - changes in core/eval.go be better viewed in spit mode

kakdeykaushik commented 1 month ago

@yashs360 implemented the changes you had mentioned. please review and lmk of any improvements. Tests(both unit and integration) are working correctly with the changes.

gauravsarma1992 commented 1 month ago

@kakdeykaushik @yashs360 The Eval attribute of the DiceCmd struct should be an interface instead of a function type. I think the current attributes of the function type are not sufficient to satisfy all future use cases. Using interfaces as the function's input and output would help provide extensibility for future changes as well. One basic distinction I see in the current system is that some commands require only the args whereas some commands require the args along with the connection information. This will also allow us to define commands in different packages as well as long as the objects satisfy the interface requirements.

JyotinderSingh commented 1 month ago

@kakdeykaushik @yashs360 The Eval attribute of the DiceCmd struct should be an interface instead of a function type. I think the current attributes of the function type are not sufficient to satisfy all future use cases. Using interfaces as the function's input and output would help provide extensibility for future changes as well. One basic distinction I see in the current system is that some commands require only the args whereas some commands require the args along with the connection information. This will also allow us to define commands in different packages as well as long as the objects satisfy the interface requirements.

Thanks for the inputs and suggestions, Gaurav. Maybe we can do that as a separate effort when it starts to become a problem instead of prematurely making it generic.

JyotinderSingh commented 1 month ago

Thanks for the contribution @kakdeykaushik, and for the review @yashs360.

Merging this.