Skip to content

Move regex check on JNumber into companion #28

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

Closed
sirthias opened this issue Jul 1, 2017 · 3 comments
Closed

Move regex check on JNumber into companion #28

sirthias opened this issue Jul 1, 2017 · 3 comments

Comments

@sirthias
Copy link

sirthias commented Jul 1, 2017

Currently the JNumber constructor always performs a regex check, even if the argument is known to never fail it (e.g. when it's called from one of the several JNumber.apply overloads).

This ticket proposes to mark the JNumber case class constructor private, add an JNumber.apply(String) overload and move the regex check into this method.

Unfortunately this fix depends on two patches to scalac (scala/scala#5730 and scala/scala#5846), which are only available as of Scala 2.11.11 (where it needs the -Xsource:2.12 flag) and 2.12.2 (no flags required) and thus won't work on Scala 2.10.

Nevertheless I think it'd be a valuable improvement that might also help to (somewhat) alleviate the concerns that @fommil has voiced in #16.

@mdedetrich
Copy link
Owner

Something similar is already being done, will see if I can integrate this. Expect an update within a couple of days

@fommil
Copy link

fommil commented Jul 1, 2017

you can always do a hand rolled class. case class is just a boilerplate generator, afterall.

@mdedetrich
Copy link
Owner

I am going to do this in a new PR since it changes source compatibility, I am likely to just use a class, no one is going to be using the JNumber constructor directly anyways

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

3 participants