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

Add instance Random Natural #73

Closed
wants to merge 1 commit into from
Closed

Add instance Random Natural #73

wants to merge 1 commit into from

Conversation

Bodigrim
Copy link
Contributor

@Bodigrim Bodigrim commented Jun 29, 2020

No description provided.

@lehins
Copy link
Contributor

lehins commented Jun 29, 2020

Dammit, I was sure I fixed that CPP stuff. It probably happened in some PR that never got merged. Oh well, I am really glad you noticed it.

There was discussion about Random instance for Natrual in #44 I was really on the edge of not defining this instance and even closed that issue, but since you already implemented it, I am OK with it to be included. After all that is exactly what Random class is for. 👍

Let's give it a day or so for someone to object, and we can merge it if no-one speaks up.

@curiousleo
Copy link
Contributor

Same comment as here: #72 (review) - I don't think we should be adding features to Random at this point. There already is a UniformRange Natural instance. If anybody wants to generate Natural values in the Word range, they can already do it.

@idontgetoutmuch
Copy link
Member

I don't think we should add this. If someone wants to generate random values for Natural then they should use UniformRange.

@Bodigrim Bodigrim closed this Feb 13, 2021
@Bodigrim Bodigrim deleted the natural branch February 13, 2021 18:37
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.

4 participants