[struct] Update StructSerializable contract (NFC) #7441
Conversation
Matches ProtobufSerializable This is necessary for templated types
| @@ -10,6 +10,7 @@ | |||
| * Marker interface to indicate a class is serializable using Struct serialization. | |||
| * | |||
| * <p>While this cannot be enforced by the interface, any class implementing this interface should | |||
| * provide a public final static `struct` member variable. | |||
| * provide a public final static `struct` member variable, or a static final `getProto()` method if | |||
There was a problem hiding this comment.
getProto
Should be getStruct, no? And what parameters should this function be allowed to take? If it can be polymorphic (eg Vector declaring a getStruct(Nat<?> rows) and Matrix declaring getStruct(Nat<?> rows, Nat<?> cols)), then there's no standard way for end code to be generic over serializable types - it'd have to hardcode based on known types, and would be unable to support arbitrary/unknown ones. In particular, it would mean epilogue would only be able to support known WPILib types, and nothing defined by users or third party libraries.
Changing this interface to require an instance method would allow for serialization of arbitrary user types. Deserialization would still need to be done through structs instantiated via static factory methods. Such a change would likely be a big lift
public interface StructSerializable<T> {
Struct<T> getStruct();
}
I missed this in #5953, which updated
ProtobufSerializableto allow a static method instead of a static variable for templated types but forgot to do the same forStructSerializablewhen templated struct support was added to the PR.