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

Allow passing precision to faker.number.int #2186

Closed
leon opened this issue May 30, 2023 · 12 comments
Closed

Allow passing precision to faker.number.int #2186

leon opened this issue May 30, 2023 · 12 comments
Labels
c: feature Request for new feature has workaround Workaround provided or linked m: number Something is referring to the number module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@leon
Copy link

leon commented May 30, 2023

Clear and concise description of the problem

In faker 7 I could specify

faker.datatype.number({ min: 30, max: 200, precision: 10 })

to get a number between 30 and 200, with an increment of 10, so: 140, 90, 180, ...

But with faker 8, the precision was removed from int.

faker.number.int({ min: 30, max: 200 })

Suggested solution

Allow passing a precision to the int faker function

faker.number.int({ min: 30, max: 200, precision: 10 })

Alternative

No response

Additional context

No response

@leon leon added c: feature Request for new feature s: pending triage Pending Triage s: waiting for user interest Waiting for more users interested in this feature labels May 30, 2023
@github-actions
Copy link
Contributor

Thank you for your feature proposal.

We marked it as "waiting for user interest" for now to gather some feedback from our community:

  • If you would like to see this feature be implemented, please react to the description with an up-vote (:+1:).
  • If you have a suggestion or want to point out some special cases that need to be considered, please leave a comment, so we are aware about them.

We would also like to hear about other community members' use cases for the feature to give us a better understanding of their potential implicit or explicit requirements.

We will start the implementation based on:

  • the number of votes (:+1:) and comments
  • the relevance for the ecosystem
  • availability of alternatives and workarounds
  • and the complexity of the requested feature

We do this because:

  • There are plenty of languages/countries out there and we would like to ensure that every method can cover all or almost all of them.
  • Every feature we add to faker has "costs" associated to it:
    • initial costs: design, implementation, reviews, documentation
    • running costs: awareness of the feature itself, more complex module structure, increased bundle size, more work during refactors

View more issues which are waiting for user interest

@ST-DDT
Copy link
Member

ST-DDT commented May 30, 2023

Related: #1596

In that issue we already approved the step feature. I keep this issue open because it is explicitly for the step feature.

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug m: number Something is referring to the number module and removed s: pending triage Pending Triage s: waiting for user interest Waiting for more users interested in this feature labels May 30, 2023
@ST-DDT ST-DDT moved this to Todo in Faker Roadmap May 30, 2023
@ST-DDT
Copy link
Member

ST-DDT commented May 30, 2023

Workaround: faker.number.int(min/10; max/10)*10

@ST-DDT ST-DDT added the has workaround Workaround provided or linked label May 30, 2023
@ST-DDT
Copy link
Member

ST-DDT commented May 30, 2023

Are you willing to create a PR for this?

@leon
Copy link
Author

leon commented May 30, 2023

Sure I can have a look

@matthewmayer
Copy link
Contributor

Note that the step feature in #1596 was for float, not for int. If we do it for one we should probably do it for all number methods.

@matthewmayer
Copy link
Contributor

Im still a bit dubious about how useful this parameter is in reality versus just multiplying by a constant as in the workaround posted by @SD-DDT .

To get a number between 30 and 200, with an increment of 10 can be clearly described by
10 * faker.number.int({ min: 3, max: 20 })
or
faker.number.int({ min: 30, max: 200, step:10 })

But this call requires looking at the documentation to see if you will get a) {10,20,30,40,50} or b) {5,15,25,35,45}
faker.number.int({ min: 5, max: 50, step:10 })

I'd rather write those two options more explicitly as:

10 * faker.number.int({min:1, max:5})
10 * faker.number.int({min:0, max:4}) + 5

@leon
Copy link
Author

leon commented May 30, 2023

After reading your comments, I did a new PR with a step param.

It feels more natural than precision, so I think we should go with that.

#2188

@matthewmayer
Copy link
Contributor

I previously argued for a name something like 'multipleOf' to make it clear it doesn't work in the same way as say the HTML input element

#1855 (comment)

@leon
Copy link
Author

leon commented Oct 18, 2023

Any progress on this?

@matthewmayer
Copy link
Contributor

Could you address the comments in the open PR: #2188 (comment)

@xDivisionByZerox
Copy link
Member

This issue has been resolved by #2586 and was released as part of https://github.com/faker-js/faker/releases/tag/v9.0.0-alpha.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature has workaround Workaround provided or linked m: number Something is referring to the number module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Todo
4 participants