Use of generics to avoid code replication #15160
muku314115
started this conversation in
General
Replies: 2 comments 1 reply
-
100% agree. Collections is meant to be a state compatible generic replacement to most of the crud operations in a module |
Beta Was this translation helpful? Give feedback.
0 replies
-
Using generics to invoke reflection is not an improvement over a couple one-line functions. The suggested change is more complicated and likely quite a bit slower. I'm sure there are other cases where generics would improve SDK code, but this is not one of them. |
Beta Was this translation helpful? Give feedback.
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Many functions perform the same steps but under different names which leads to code replication.
This process can be simplified by passing generics types and hence making more generalised functions in the project.
Eg. under x/staking/types/keys.go
func GetValidatorKey(operatorAddr sdk.ValAddress) []byte {
return append(ValidatorsKey, address.MustLengthPrefix(operatorAddr)...)
}
func GetValidatorByConsAddrKey(addr sdk.ConsAddress) []byte {
return append(ValidatorsByConsAddrKey, address.MustLengthPrefix(addr)...)
}
The above functions perform more or less the same task but with a slight difference, this can be changed to a more generic function like :-
Approach 1
func InterfaceKeyMapFunc[K sdk.ValAddress | sdk.ConsAddress](addr K) []byte {
switch reflect.TypeOf(addr).String() {
case "types.ValAddress":
return ValidatorsKey
case "types.ConsAddress":
return ValidatorsByConsAddrKey
}
return nil
}
func GetValidatorKeyGeneric[K sdk.ValAddress | sdk.ConsAddress](addr K) []byte {
return append(InterfaceKeyMapFuncK, address.MustLengthPrefix(addr)...)
}
**Approach 2 - adding a map **
InterfaceKeyMap = map[string][]byte{"types.ValAddress": ValidatorsKey, "types.ConsAddress": ValidatorsByConsAddrKey}
func GetValidatorKeyGenericMap[K sdk.ValAddress | sdk.ConsAddress](addr K) []byte {
return append(InterfaceKeyMap[reflect.TypeOf(addr).String()], address.MustLengthPrefix(addr)...)
}
the values will be constants in a separate file altogether, strings are being used here just to propose the idea.
Would like to get some thoughts on this approach and if it can be beneficial/detrimental in the future considering the shift may cause confusion
Beta Was this translation helpful? Give feedback.
All reactions