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

Make BigInteger and BigDecimal parsing lazy #828

Merged
merged 1 commit into from Oct 23, 2022

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Oct 23, 2022

@cowtowncoder please feel free to reject this - but this relatively small change might be useful for v2.14

This is useful for rallyhealth/weePickle#118

This change means that JsonParser.getNumberType will not need to parse the BigInteger but if you later need the BigInteger (getBigIntegerValue() call), it will then be parsed (just once).

It may also be useful to do something similar with BigDecimal parsing but that change is not as immediately useful.

@cowtowncoder
Copy link
Member

@pjfanning Excellent! I am thinking I do need to release 2.14.0-rc3, but these are important improvements I think so that's fine. And yes, testing might show that similar would make sense for BigDecimal, but one step at a time.
I hope this merges cleanly to master but we'll see.

I hope testing we have is able to find edge cases there might be but... we'll see.

@cowtowncoder cowtowncoder changed the title make BigInteger parsing lazy Make BigInteger parsing lazy Oct 23, 2022
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Oct 23, 2022
@cowtowncoder cowtowncoder merged commit 351de07 into FasterXML:2.14 Oct 23, 2022
cowtowncoder added a commit that referenced this pull request Oct 23, 2022
@cowtowncoder
Copy link
Member

@pjfanning One thing we could do eagerly would to add accessor (I renamed one here to _getBigInteger(), minor naming convention thing) for BigDecimal too; even if at first it'd still be eagerly accessed.
So something like

    protected BigDecimal _getBigDecimal() { return _numberBigDecimal; }

in hindsight these probably should have been there from beginning.

@pjfanning
Copy link
Member Author

@cowtowncoder I've made big decimal lazy too and renamed the private methods

@pjfanning pjfanning changed the title Make BigInteger parsing lazy Make BigInteger and BigDecimal parsing lazy Oct 23, 2022
@cowtowncoder
Copy link
Member

@pjfanning Is there a PR? I merged this one earlier.

@pjfanning
Copy link
Member Author

@cowtowncoder I created #830 - I hadn't noticed that this PR was merged and thought that my changes in the related branch would update this PR

@cowtowncoder
Copy link
Member

Ah. Thank you for creating a new PR, I think updates after merge may not be visible.

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.

None yet

2 participants