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

Valid number string predicates #379

Merged
merged 4 commits into from
Dec 22, 2017

Conversation

kusamakura
Copy link
Contributor

Adds the following string predicates: ValidInt, ValidLong, ValidDouble, ValidBigInt, and ValidBigDecimal.

I initially implemented this with a typeclass-based approach, e.g. ParsableString[N], which the Validate instance would use, but it seemed to be double the work than just creating new predicates and the corresponding Validate instances. Both seem extensible enough, though I surmise that a typeclass-based approach would be a little less verbose, since supplying the typeclass instances would be enough to get started. @fthomas, if you think that that's a more sensible approach, then I can always update this PR.

I'm also thinking of adding inference instances for these, but I'll hold off for now until I can find a good way of doing it without copious amounts of copy and paste.

@codecov
Copy link

codecov bot commented Dec 21, 2017

Codecov Report

Merging #379 into master will increase coverage by 0.23%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #379      +/-   ##
==========================================
+ Coverage   96.38%   96.62%   +0.23%     
==========================================
  Files          41       41              
  Lines         498      503       +5     
  Branches       11       10       -1     
==========================================
+ Hits          480      486       +6     
+ Misses         18       17       -1
Impacted Files Coverage Δ
...red/src/main/scala/eu/timepit/refined/string.scala 100% <100%> (ø) ⬆️
...c/main/scala/eu/timepit/refined/cats/package.scala 100% <0%> (+33.33%) ⬆️

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 41e8946...40c0cc9. Read the comment docs.

@fthomas
Copy link
Owner

fthomas commented Dec 22, 2017

Thanks @kusamakura, this looks great!

The ParseableString[N] approach is an interesting idea. Instead of defining a predicate and the Validate instance you just need to define the ParseableString instance. But as you already noted, the latter is just a little bit less work and probable a little bit harder to understand.

@fthomas fthomas merged commit 96897dc into fthomas:master Dec 22, 2017
@kusamakura kusamakura deleted the topic/parseable-predicates branch December 23, 2017 05:02
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.

2 participants