checked constructor for potential error conditions

[mroth]

This commit introduces a variant of the NewChooser constructor that will pre-check for and error on conditions that could later cause a runtime issue during Pick.

The conditions handled are a lack of valid choices (fairly preventable by consumers) and a potential integer overflow in the running total (far more subtle, but an edge case). In general, it is highly preferable for us to expose these predictable scenarios rather than potentially allowing a runtime error that could panic or produce invalid results. I was thinking of this Cloudflare blog post as a similar scenario when proposing these changes.

The initial version of this is API backwards compatible by introducing a new NewChooserChecked function and changing the behavior of the NewChooser to panic at construction time in invalid states. However I'd like to iterate towards a more simple API before making this change. Some options:

1. Expose both NewChooser and NewChooserChecked (as in the current proof of concept commit). I am not generally in favor of this as it introduces more user-complexity than needed and the distinction here is not significantly subtle to require the cognitive overhead of both options.

2. Privatize newChooserChecked and keep the panic on error behavior on NewChooser. Pros: API backwards compatibility (though pre-release, so not as important), simpler caller semantics, the error conditions are currently fairly edge and unlikely to be encountered in typical usage. Cons: doesn't force error handling, and the overflow error scenario in particular, being an edge case, is unlikely to be pre-checked by consumers of the API, and panic recovery is a pain.

3. Make NewChooserChecked the new NewChooser (renaming/replacing it), which introduces an API change in the return value of the constructor to (*Chooser, error). This would require a semantic version bump, but we're still in pre-release versioning so it makes sense to iterate towards the most ideal API now while we have the flexibility. Pros: Forces dealing with the potential error condition. Cons: Forces slightly more complex initialization code everywhere (e.g. checking err), current error conditions are not incredibly likely to emerge in typical usage.

Currently my current leaning is towards option number 3 (especially since the potentially use cases of this library include problem domains with highly reliable characteristics are desirable, such as load-balancing) but I would love to hear from actual users of this library. (To best have an informed opinion, I suggest reviewing the actual source code changes so you can see how likely you are to encounter the potential error conditions in your own usage.)

Some additional API considerations:

• If going with option 1 or 3, whether to additionally export the sentinel error types (e.g. ErrWeightOverflow) or not (currently exported in initial commit). My initial inclination is that exporting them complicates the external surface of the API with minimal benefit, and I cannot currently conceive of a situation where a caller fo the constructor would act upon different error conditions significantly differently. (please let me know if you can think of a need here!).
• There are some additional minor unrelated API changes I will want to consider making for ergonomics, so if this ends up going that route, I'll open additional issues to discuss those and they can potentially be rolled into a single breaking version bump.

[mroth]

Pinging @0xERR0R since from a quick GitHub search, your Blocky project appears to be the current largest public project utilizing this library.

Pinging @mccutchen since I noticed you starred this repo recently and we've worked together in the past. :grin:

[0xERR0R]

I would prefer the option 3. It is always better to check all errors earlier and fail fast and cheap (and not at runtime).

[codecov[bot]]

Codecov Report

Merging #4 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master        #4   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           22        27    +5     
=========================================
+ Hits            22        27    +5     

Impacted Files	Coverage Δ
weightedrand.go	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3b00289...5da14cc. Read the comment docs.

[mroth]

@0xERR0R potentially up for a quick code review on this before merge?

I'm thinking this could be targeted for a shortly upcoming v0.4 release after examining any other desirable API changes.