Replace "expected" by more precise terms #739
Conversation
* Replace the vague term "expected" with the RFC 2119-compatible terms "should", "must", "required" * Clarify that an error must be thrown if an integer cannot be represented losslessly
ChristianSi
changed the title
|
LGTM! |
README.md
Outdated
| handled losslessly. If the result of parsing a value with unlimited precision | ||
| would result in an integer that cannot be represented losslessly, an error must | ||
| be thrown. |
There was a problem hiding this comment.
This feels like a very complex sentence -- I understand what this means, but it needed a couple of readings and conscious thinking to figure it out.
I think we can drop the "unlimited precision" part of this sentence without losing any of the main benefits. :)
| handled losslessly. If the result of parsing a value with unlimited precision | |
| would result in an integer that cannot be represented losslessly, an error must | |
| be thrown. | |
| handled losslessly. If an integer that cannot be represented losslessly, | |
| an error must be thrown. |
There was a problem hiding this comment.
@pradyunsg I agree to an extend. Though as advocate for the devil, a very literal reading can then become: I parse an int with Int64.TryParse and get back -2. Fine, right?
Nope, because that function would give a 2's complement result, input could've been 2^63 + 2. Hence I introduced the wording "with unlimited precision".
I would highly recommend that at some point we diverge into two documents: one for the general public, and one being the actual spec using precise spec wording. The second one intended for implementers, who are expected to be able to read such more strict documents.
There was a problem hiding this comment.
Hmmm... I'm pretty sure I'd consider 2^63 + 2 becoming -2 as a case of "cannot be represented losslessly".
I would highly recommend that at some point we diverge into two documents: one for the general public, and one being the actual spec using precise spec wording. The second one intended for implementers, who are expected to be able to read such more strict documents.
Perhaps? I understand the balance we're trying to strike here, and if it's definitely-not-possible, we should do this. I'm not convinced that it'd be a worthwhile exercise right now though. :)
There was a problem hiding this comment.
PS: I think the devil is pretty hunky-dory right now. It'll be fine if you don't advocate for them for a bit. :P
There was a problem hiding this comment.
I'm not convinced that it'd be a worthwhile exercise right now though. :)
Agreed, I didn't mean "right now", but more for a vNext ;)
|
@pradyunsg Your suggestion was not quite clear, but I've just pushed a commit that (I think) fulfills it. Better now? @abelbraaksma To me the rewritten sentence still sounds fairly unambiguous, though it's true that the old version was even more explicit. |
|
What's the status of this? @pradyunsg Are you ready to merge? |
|
I thought I had already! |
Fixes #736.