Support deserialization of double aliases from strings #2660
Conversation
Generate changelog in
|
| default: | ||
| throw new IllegalArgumentException("Cannot deserialize string into double: " + value); | ||
| try { | ||
| double doubleValue = Double.parseDouble(value); |
There was a problem hiding this comment.
Why not just call Double.parseDouble? Because we don't want to accept things like hexadecimals and 1e10 (but we can accept 1.0E10 because that's a properly formatted double).
There was a problem hiding this comment.
Maybe add this as a comment on the codegen code? I could certainly see this be asked in a couple of years and someone changing this because they fail to find your PR comment
✅ Successfully generated changelog entry!What happened?Your changelog entries have been stored in the database as part of our migration to ChangelogV3. Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. |
| default: | ||
| throw new IllegalArgumentException("Cannot deserialize string into double: " + value); | ||
| try { | ||
| double doubleValue = Double.parseDouble(value); |
There was a problem hiding this comment.
Maybe add this as a comment on the codegen code? I could certainly see this be asked in a couple of years and someone changing this because they fail to find your PR comment
| default: | ||
| throw new IllegalArgumentException("Cannot deserialize string into double: " + value); |
There was a problem hiding this comment.
Why is this specific to map keys? How are we deserializing non-map-key double aliases if not through this code? (I'm asking specifically because I'm curious about the perf cost of this change - it might not matter if it's for double alias map keys, but maybe more so if that's for all double alias deserializations?)
| bearertokens: map<BearerTokenAliasExample, ManyFieldExample> | ||
| integers: map<IntegerAliasExample, ManyFieldExample> | ||
| # doubles: map<DoubleAliasExample, ManyFieldExample> # typescript freaks out with the 'NaN' | ||
| doubles: map<DoubleAliasExample, ManyFieldExample> # typescript freaks out with the 'NaN' |
There was a problem hiding this comment.
nit: I would probably just remove the typescript comment too, as I'm unclear what value it provides in the conjure-java repo
| doubles: map<DoubleAliasExample, ManyFieldExample> # typescript freaks out with the 'NaN' | |
| doubles: map<DoubleAliasExample, ManyFieldExample> |
Before this PR
Aliases of doubles are valid keys in maps. Keys in maps are serialized as strings. Aliases of doubles can only be deserialized from
NaN,Infinity, and-Infinity. We should support deserialization of other valid double values.After this PR
==COMMIT_MSG==
Support deserialization of double aliases from strings
==COMMIT_MSG==
Possible downsides?