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

roman-numerals: Rewrite to return a Maybe. #304

Closed
rbasso opened this issue Sep 18, 2016 · 2 comments
Closed

roman-numerals: Rewrite to return a Maybe. #304

rbasso opened this issue Sep 18, 2016 · 2 comments

Comments

@rbasso
Copy link
Contributor

rbasso commented Sep 18, 2016

Considering what was discussed in #194 (comment), we should probably rewrite roman-numerals to make it more idiomatic.

Currently, the reference json file only suggests test for values between 1 and 3000.

Should we really add new tests for:

  • Negative values?
  • Zero?
  • Numbers greater than 3000?

Or should we just change the return type to a Maybe, leaving to the user to choose how it will handle those cases?

@petertseng
Copy link
Member

All the description.md says is "There is no need to be able to convert numbers larger than about 3000.", has not mentioned lower limits. I assume we could test it.

Questions to ask:

  • If we change to Maybe and add a test for 0, would we expect it to be Nothing or Just ""?
  • If we change to Maybe and add a test for -1, would we expect it to be Nothing or Just "-I"?

I don't know if there is historical basis for the proposed Just answers so really I am making things up. If you asked me what I would pick, I'd pick Nothing for both of those, and also a Nothing for 4000.

Or should we just change the return type to a Maybe, leaving to the user to choose how it will handle those cases?

Interesting, we haven't done it before. It could be a worthy experiment. You may recall at #292 I wondered whether people would get confused by that, but maybe if we told them "it's up to you!" then people need not be confused.

@rbasso
Copy link
Contributor Author

rbasso commented Sep 20, 2016

If we change to Maybe and add a test for 0, would we expect it to be Nothing or Just ""?

For historical reasons, I would prefer to use Nothing. Nulla would probably be too much for the sake of correctness. 😄

Interesting, we haven't done it before. It could be a worthy experiment. You may recall at #292 I wondered whether people would get confused by that, but maybe if we told them "it's up to you!" then people need not be confused.

I would favor this approach, using a note in HINTS.md.

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

No branches or pull requests

2 participants