search

HdrHistogram / hdrhistogram-go

A pure Go implementation of Gil Tene's HDR Histogram.
MIT License
429 stars 64 forks source link

Export/Import Histograms #5

Closed tylertreat closed 9 years ago

tylertreat commented 9 years ago

Thanks for the golang port, super useful.

This is a stab at addressing what was discussed in issue #1. I think the consensus from that discussion was to keep the API simple. Any kind of Histogram serialization should be deferred to users of the library, but a way to export a view of the Histogram should be provided.

This introduces a Snapshot which is produced on Export. This allows users to do what they want with it and later reconstruct a Histogram from it using Import. Another item is the Equals method which is used to compare a Histogram for equality.

Also changed New so that the sigfigs argument is an int64, which reflects the type on the underlying Histogram.

codahale commented 9 years ago

(Sorry, just realized I hadn’t responded to this!)

This looks good to me except for the change to New. The only reason sigfigs is stored as an int64 internally is to avoid a handful of type conversions. The use of a 64-bit value for an argument which is typically in [3,5] exposes a purely internal concern via the API, which I’d rather not do.

Other than that, I’m happy with the PR. Thanks!

tylertreat commented 9 years ago

And fixed.

codahale commented 9 years ago

Awesome! Thank you!