[
https://issues.apache.org/jira/browse/CASSANDRA-8385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15296975#comment-15296975
]
Joshua McKenzie commented on CASSANDRA-8385:
--------------------------------------------
* AbstractType.toJsonString includes protocolVersion, however I did not see
that parameter used in any of the implementers.
* For the various valueLengthIfFixed implementations, I think it might make
sense to relax scope on TypeSizes and reference them instead of us encoding
that in 2 separate places.
* The naming of "CollectionType.Kind.type" doesn't seem clear to me (fully
acknowledging that you simply made a method of what was previously a member
variable with the same name). While we have "Kind"'s littered throughout the
code-base, the meaning of "a type of Kind of CollectionType" doesn't give a lot
of helpful information to reason about and I think this is an opportunity to
fix at least one instance of a widely overloaded name in the code-base. Perhaps
naming it to "classType" to at least denote the context of the type in the name?
* Since neither {{CollectionType}} nor {{ConcreteType}} have implementatoins of
{{getSerializer}}, I don't follow the comment in {{ConcreteCollectionType}} of
providing a more specific type than the one in either to resolve a type
conflict.
* It looks like we're losing compile-time type-checking in
{{ConcreteType.decompose}} relative to our previous implementation in
{{AbstractType.decompose}}. We're still protected at runtime via {{type.cast}}
and, in general, I think the pros outweight the cons here, but thought I'd
point it out in case I was misunderstanding something here.
* {{ConcreteType.isFrozenCollection}} is unused.
{{CreateIndexStatement.validate}} is likely where this method was intended to
be referenced as its duplicating this logic.
* The javadoc for the ConcreteType could use some elaboration compared to just
re-using the old javadoc from AbstractType as it is no longer accurate nor
representative of the fullness of what the class does.
* nit: Some grammatical errors in comment for {{AbstractType.compareForCQL}}.
Should read: "this ignore(s) ReversedType", "to be use(d)"
* nit: import order in ConcreteType.java is off
> Clean up generics in uses of AbstractType
> -----------------------------------------
>
> Key: CASSANDRA-8385
> URL: https://issues.apache.org/jira/browse/CASSANDRA-8385
> Project: Cassandra
> Issue Type: Improvement
> Components: Local Write-Read Paths
> Reporter: Branimir Lambov
> Assignee: Branimir Lambov
> Priority: Trivial
> Attachments: 8385.patch
>
>
> Almost all uses of AbstractType are from code that doesn't know or care what
> the specific type is and would be better served by a non-generic version of
> the concept.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
