feat: Panic -> Error migration #555
Conversation
message.go
Outdated
| var defaultErrorStrMapSlice = make([]map[string]string, 0) | ||
| var defaultErrorIntMap = make(map[string]int64) | ||
| var defaultErrorKeyValues = KeyValues{} | ||
| var defaultErrorKeyZScores = KeyZScores{} |
There was a problem hiding this comment.
On retrospect this might not be a great idea at all
Maybe I misunderstood the proposal a little
There was a problem hiding this comment.
We don't need to pre-define these default error values. We should just return a zero value of the corresponding type as well as the error. For example:
return defaultErrorRedisMessageSlice, fmt.Errorf("redis message type %s is not a array", typeNames[typ])
should be just
return nil, fmt.Errorf("redis message type %s is not a array", typeNames[typ])
There was a problem hiding this comment.
@rueian Zero values for the non-nullable ones?
It will not compile in those cases when we send back a nil for non-nullable one I guess
Okay let me do try this refactor
There was a problem hiding this comment.
Also, is this the only file where we require changes for this? @rueian
Any other changes we require other than the tests
There was a problem hiding this comment.
These are zero values: https://yourbasic.org/golang/default-zero-value/
There was a problem hiding this comment.
Yes, the message.go is the only file that needs to be changed other than the tests.
This is to address #366
This is in draft state with some idea I understood from the current proposal thread.
We can discuss and decide on what specifically we want to do here, and how much of it has to stay backwards compatible and in what all way and whether we want to introduce some breaking major changes too.
@rueian Let's continue the discussion further on this