[CASSANDRA-5744] Cleanup AbstractType/TypeSerializer classes

• Type: Improvement
• Status: Resolved
• Priority: Low
• Resolution: Fixed
• Fix Version/s: 2.0 beta 2
• Assignee: Sylvain Lebresne
• Reporter: Sylvain Lebresne

Authors

Sylvain Lebresne

Reviewers

Carl Yeksigian

• Created: 10/Jul/13 14:54
• Updated: 16/Apr/19 09:32
• Resolved: 12/Jul/13 13:39

Description

This is somewhat a followup to CASSANDRA-4495. I'm attaching 3 patches that do the following:

1. It inverse the serialize and deserialize method in TypeSerializer. Sorry I didn't saw that earlier, but calling serialize the method that takes a byte array to produce an object feels wrong to me (and wikipedia seems to agree with me that this should the other way around: http://en.wikipedia.org/wiki/Serialization )
2. For historical reasons (which imo were somewhat of a mistake in the first place but that's another story), we accept an empty byte buffer as a valid value for any type. When I say "valid", I mean that validate() never throw (except for InetAddressType as it happens, but that's more of an inconsistency that the patch fixes). However, for some reason most deserialize methods were just throwing a random exception if an empty byte buffer. So I think we should be coherent here, if validate() pass, you should be able to deserialize the value alright, and the 2nd patch make sure of that (return null when there was nothing else making sense).
3. The patch removes a bunch of code duplication. Namely, AbstracType has a getSerializer() method that return the corresponding TypeSerializer, but despite that, every AbstractType subclass was redefining its compose, decompose and validate that were just calling the corresponding method in their deserializer. So the patch makes those method concrete in AbstractType and remove the code duplication all over the place. Furthermore, TypeSerializer had a getString(ByteBuffer) and a toString(T value) methods. But since we also have a deserialize(ByteBuffer), the former getString() is really not useful as it's just toString(deserialize()). So the patch also remove that method.