You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Empty string is not a valid representation of a decimal number and should not be accepted.
This might be correct in a mathematcical sense, but it totally ignores the reality of parsing!
It is very common in the context of data handling to omit 0s in data representations like CSV or alike. So imho you can ask 100 people and more than 99 would answer that zero should be the equivalent when converting "nothing".
Imho you should respect Postel's law here. Again: It is about parsing, not serialization!
At least this should be marked as an serious behaviour chaning issue.
Some of the reasoning is correct, but it's applied at incorrect place. If your domain requires handling of empty strings as zeroes, you need to apply your custom logic, e.g. replace empty input with zero, before you get to this method.
Zero is not special here. It is equally incorrect to return zero from an empty string as it would be to return Random.nextDouble. Your domain is special? Brilliant, then apply your domain logic at the domain level.
You are right: Zero is not special in the domain of parsing ;-) So I consider this to be a part of the convenient user API of this lib.
Let's commit, that we have different opinions of this topic.
Then let us just step aside from wrong or right and try to focus on the API of a library and the constraints and responsabilities it has: Imho as a maintainer you are resonsible to keep the API (including behaviour!) as stable as you can. This was a huge change and imho it would have been better to balance pros and cons more carefully.
The consequence of your observation is: Someone who needs to be aware of empty strings would have to encode special code before calling this function.
Now the other way round is needed - as I proclaim: The waste majority would have to change this now!
Let us recap other alternatives:
it would have been possible to add some strict mode like parseStringStrictly -> solution for all mathematics
it would be possible to add some "error handling strategy" like the .NET does for Charset encoding iirc. parseString(..., ..., errorHandle: ErrorHandle), where the default is either "make zero" or "throw exception" depending on the stand-point.
or quite similar to above but perhaps more kotlinesque: parseStringOrDefault and/or parseStringOrElse
To be fair: Obviously noone besides me cared about this change, even though it is much more severe than the fixing procedure would unveil ;-)
Thank you for your opinion on this topic, while I disagree with you on most points, I can understand that it can be irritating when something that was working suddenly breaks. Unfortunately in this case I agree completely with @okarmazin, the solution should be applied at the domain level.
Activity
ionspin commentedon Feb 2, 2023
Thanks for reporting!
Bump kotlin to 1.8.20. Empty string is not valid floating point numbe…
Lysander commentedon Mar 21, 2024
This might be correct in a mathematcical sense, but it totally ignores the reality of parsing!
It is very common in the context of data handling to omit
0
s in data representations like CSV or alike. So imho you can ask 100 people and more than 99 would answer that zero should be the equivalent when converting "nothing".Imho you should respect Postel's law here. Again: It is about parsing, not serialization!
At least this should be marked as an serious behaviour chaning issue.
okarmazin commentedon Mar 21, 2024
Some of the reasoning is correct, but it's applied at incorrect place. If your domain requires handling of empty strings as zeroes, you need to apply your custom logic, e.g. replace empty input with zero, before you get to this method.
Zero is not special here. It is equally incorrect to return zero from an empty string as it would be to return
Random.nextDouble
. Your domain is special? Brilliant, then apply your domain logic at the domain level.Lysander commentedon Mar 21, 2024
You are right: Zero is not special in the domain of parsing ;-) So I consider this to be a part of the convenient user API of this lib.
Let's commit, that we have different opinions of this topic.
Then let us just step aside from wrong or right and try to focus on the API of a library and the constraints and responsabilities it has: Imho as a maintainer you are resonsible to keep the API (including behaviour!) as stable as you can. This was a huge change and imho it would have been better to balance pros and cons more carefully.
The consequence of your observation is: Someone who needs to be aware of empty strings would have to encode special code before calling this function.
Now the other way round is needed - as I proclaim: The waste majority would have to change this now!
Let us recap other alternatives:
parseStringStrictly
-> solution for all mathematicsparseString(..., ..., errorHandle: ErrorHandle)
, where the default is either "make zero" or "throw exception" depending on the stand-point.parseStringOrDefault
and/orparseStringOrElse
To be fair: Obviously noone besides me cared about this change, even though it is much more severe than the fixing procedure would unveil ;-)
ionspin commentedon Mar 21, 2024
Hello @Lysander,
Thank you for your opinion on this topic, while I disagree with you on most points, I can understand that it can be irritating when something that was working suddenly breaks. Unfortunately in this case I agree completely with @okarmazin, the solution should be applied at the domain level.
Once again, thanks for the opinions!
Lysander commentedon Mar 21, 2024
Thanks for the overall excellent library ;-)
No problem - let's embrace different opinions discussed without harm or even violence. There is inherent value in this - especially nowadays :-)