[
https://issues.apache.org/jira/browse/CASSANDRA-9705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14628001#comment-14628001
]
Benedict commented on CASSANDRA-9705:
-------------------------------------
I have not reached the main guts of the changes yet. A few initial comments, as
they've reached sufficient critical mass. I apologise if some of this stuff is
blurring the lines of scope with 8099, but I want to raise things as they occur
to me.
* {{ColumnData}} and {{ComplexColumnData}} are a bit confusingly named, to my
mind, at least, because it is {{(Column x Row) Data}}. Especially since
{{Cell}} implements {{ColumnData}} but is also a _component_ of a
{{ComplexColumnData}} in which context it doesn't seem very reasonable to call
it a {{ColumnData}} at all. Could we not just have {{ComplexCell}} (made up of
{{Cell}}), both implementing {{AbstractCell}}; or a {{SimpleCell}} both
implementing {{Cell}}?
* Is it really better to have {{Row}} implement {{Iterable<ColumnData>}} than
{{Iterable<Cell>}}? It looks like we use the latter more often, and have a lot
of new expansions of for-loop into uglier while-loop-with-Iterator. Perhaps
whichever we opt to not make default, we should return an {{Iterable}} for the
opposing method, to help retain clarity? With Java 8 it is not at all ugly, and
most likely also of zero runtime cost.
* I like the migration to {{Builder}}, but we still have a lot of methods
labelled {{writeX}}
* There are some unused imports around the place
* {{SSTableReversedIterator:365}}: double assignment of {{markerEnd}}
* {{SSTableReversedIterator.ReverseIndexedReader}}: {{slice}} effectively moves
the outer class itself, and duplicates/mirrors some of its functionality. Could
we not merge the behaviour, so that slice() returns {{this}}, or another
instance of the same class?
** Same applies to {{SSTableIterator.ForwardIndexedReader}}, and to some extent
the distinction between {{ForwardReader/ForwardIndexedReader}} and
{{ReverseReader/ReverseIndexedReader}}.
* {{CompositesSearcher:176}}: unused property
* {{ObjectSizes:44}}: unused property
> Simplify some of 8099's concrete implementations
> ------------------------------------------------
>
> Key: CASSANDRA-9705
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9705
> Project: Cassandra
> Issue Type: Sub-task
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Fix For: 3.0 beta 1
>
>
> As mentioned in the ticket comments, some of the concrete implementations
> (for Cell, Row, Clustering, PartitionUpdate, ...) of the initial patch for
> CASSANDRA-8099 are more complex than they should be (the use of flyweight is
> typically probably ill-fitted), which probably has performance consequence.
> This ticket is to track the refactoring/simplifying those implementation
> (mainly by removing the use of flyweights and simplifying accordingly).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
