Skip to content

Ready: Time Series API#543

Merged
alexmoore merged 139 commits intodevelopfrom
timeseries
Dec 14, 2015
Merged

Ready: Time Series API#543
alexmoore merged 139 commits intodevelopfrom
timeseries

Conversation

@alexmoore
Copy link
Contributor

Finally ready. Contains all the new classes/commands/operations for the new TS API.
Also contains some general code formatting cleanup, as well as removing support for Java 6.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to declare as static

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logger?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For better readability it may be useful to introduce a factory method:

Cell.create()

And maybe move all the magic inside it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. That is a pretty verbose line already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then our Cell knows about every network protocol implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right about this case, but at the moment, since we have the only one protocol from my point of view it's better to introduce template method. Later, if that become a case, we may do simple refactoring to introduce AbstractFactory with PBCellConcreteFactory and WhateEverProtoConcreteFactory..

if we are going to move magic inside the factory method, better to call the method kinda fromPB(..)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reduce memory consumption, since it is stateless and without polymorphic behavior, better to use as a kind of static facade.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to instantiate knownParams here, since we know exact size

alexmoore and others added 22 commits December 5, 2015 00:51
Rename & split ImmutablePbResultFactory into PbResultFactory and
CollectionConverters.

IntelliJ Reformat, & cleanup.
…-creation

Simplify Riak cluster creation HostAndPort was added
Ready: TS List Keys + Lite/Immutable TS data classes.
@alexmoore alexmoore changed the title Time Series API Ready: Time Series API Dec 11, 2015
@srgg
Copy link
Contributor

srgg commented Dec 12, 2015

+1 after my last cleanup a6e48fb

alexmoore added a commit that referenced this pull request Dec 14, 2015
@alexmoore alexmoore merged commit b9b7f77 into develop Dec 14, 2015
@alexmoore alexmoore deleted the timeseries branch March 29, 2016 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants