Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Range fields public so others can impl SampleRange #114

Closed
wants to merge 1 commit into from

Conversation

walruscow
Copy link

This allows external types to implement SampleRange and use Range. Currently it is impossible to implement construct_range because the fields are private and a Range struct cannot be created.

Alternative approaches would be to create another new() function on Range, or to not export SampleRange & co.

This allows external types to implement SampleRange and use Range.
Currently it is impossible to implement construct_range because the
fields are private and a Range struct cannot be created.
@bhickey
Copy link

bhickey commented Aug 26, 2016

I think we should avoid leaking the internals of Range, especially implementation details like accept_zone (which is eliminated in PR #115).

@walruscow
Copy link
Author

From the look of it, #115 really just renames accept_zone, but could actually remove it (leading_zeros() oughtn't to be an expensive computation to perform per-call).

I agree that my approach here seems wrong (mostly due to the presence of accept_zone), but it enables this range feature to interoperate with other crates, which appears to be the original intention.

I wanted to reuse this crate's random range interface for another integer type I have created, just like I can use the rng.gen() function. I assumed that was intended, since SampleRange is publicly exposed, and the documentation mentions other types.

Is this inconsistency an oversight or a technical limitation of the language?

@walruscow walruscow closed this Sep 16, 2016
@aldanor
Copy link

aldanor commented Feb 6, 2017

Was also wondering what's the intended way of implementing SampleRange for user types.

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.

3 participants