Support get/is_registered methods for serde #1122
Conversation
|
|
||
| // IsIDRegistered returns true if the given schema ID is registered and has | ||
| // a decode function available. | ||
| func (s *Serde) IsIDRegistered(id int) bool { |
There was a problem hiding this comment.
What's the need for this, when you have ForEncoding and ForDecoding below?
There was a problem hiding this comment.
I want to check if we support schema or not before decoding, and if we do not support it, implement logic to get a new schema or something
There was a problem hiding this comment.
Given that, are the getters needed?
| func (s *Serde) IsIDRegisteredForEncoding(id int) bool { | ||
| ids := s.loadIDs() | ||
| t := ids[id] | ||
| return t.exists |
There was a problem hiding this comment.
This doesn't check if encode or appendEncode are non-nil?
| // IsTypeRegistered returns true if the given type is registered and has | ||
| // an encode function available. | ||
| func (s *Serde) IsTypeRegistered(v any) bool { | ||
| t, ok := s.loadTypes()[reflect.TypeOf(v)] |
There was a problem hiding this comment.
Since types are only used for encoding, is there any reason not to move this to checking just IsIDRegisteredForEncoding?
| return ok && (t.encode != nil || t.appendEncode != nil) | ||
| } | ||
|
|
||
| // GetRegisteredIDs returns a slice of all registered schema IDs (for encoding). |
There was a problem hiding this comment.
Why is this specific to just encoding? Naming is inconsistent with the other getter.
| return result | ||
| } | ||
|
|
||
| // GetRegisteredTypes returns a slice of all registered types. |
There was a problem hiding this comment.
What's the need for this? This seems like some internal niche guts
There was a problem hiding this comment.
Mainly, I need only IsIDRegistered and GetRegisteredIDs; this I implemented additionally
Dear maintainer.
As a Developer, I want to check if the schema is present or not. I added methods to do it.
I will be happy to hear your feedback and fix comments if you have any.