Ready: TS List Keys + Lite/Immutable TS data classes.#579
Ready: TS List Keys + Lite/Immutable TS data classes.#579alexmoore merged 18 commits intotimeseriesfrom
Conversation
… QueryResult more flexible for input
There was a problem hiding this comment.
IMO better to introduce complete Abstract Factory pattern in case we decide to move forward with both mutable and immutable APIs
There was a problem hiding this comment.
Yeah, that's the way that class seems to be headed.
Rename & split ImmutablePbResultFactory into PbResultFactory and CollectionConverters. IntelliJ Reformat, & cleanup.
|
Per basho/riak_kv#1291, there will be no non-streaming version of list_keys for TS. Apologies if you guys are already aware and dealing with this. |
|
@srgg Looks like a convertible iterator won't work, the protobuf library iterates over the collection twice: https://github.com/google/protobuf/blob/v2.5.0/java/src/main/java/com/google/protobuf/AbstractMessageLite.java#L323 https://github.com/google/protobuf/blob/v2.5.0/java/src/main/java/com/google/protobuf/AbstractMessageLite.java#L329 I have some time tonight, I'll see if I can switch it to a convertable iterable so we can get it in. |
|
@alexmoore Damn... will also take a look at it |
There was a problem hiding this comment.
Shall we refactor this logic to Convertible one?
There was a problem hiding this comment.
So a ConvertableIteratorIterator ?
There was a problem hiding this comment.
Sounds like fun, we should look into Guava in the future.
There was a problem hiding this comment.
@srgg Actually I think it's better to copy here for now. If we do a ConvertableIteratorIterator we'll still have to copy it to a list for the QueryResult constructor, or completely rewrite QueryResult to use an Iterator under the covers. Thoughts?
|
+1 with that change #579 (comment) |
|
I can't wait until we're JDK8+ and we can use the streaming apis. |
|
May be it makes sense to move anything related to the Magic Iterable to the specific packge |
|
👍 Tests pass and looks like your tests provide good coverage of the effected code. |
|
Thanks for the review @christophermancini & @srgg. |
Ready: TS List Keys + Lite/Immutable TS data classes.
No description provided.