Draft
Conversation
aszlig
force-pushed
the
proptests
branch
2 times, most recently
from
4e3e1a5 to
80a188c
Compare
First of all, the return type of the function is Result, which is not needed because we can *always* generate a roman number from any u64. Second, the input is allowed to be zero and since this is not representable, the implementation so far has returned an empty string. Given that the return value is Result and the input can be zero, one could possibly assume that whenever we pass a zero we get an Err, which is not the case. All this is really not necessary since we now use the NonZeroU64 type which removes possible failures by design in our function. Signed-off-by: aszlig <aszlig@nix.build>
This is mainly to be in par with the changes to arabic2vinculum, which has changed its argument from u64 to NonZeroU64. Since vinculum2arabic is the inverse of the former, we should also reflect that in returning the same type. Signed-off-by: aszlig <aszlig@nix.build>
This adds a few tests based on some assumptions based on properties of reman numerals, for example that in subtractive notation we should never have more than three of the same consecutive characters. Right now these tests are very basic because in order to get more exhaustive we need to have access to some kind of mapping between roman digits and its values. Our current tests only cover properties that hold true without knowing more intrinsic details. Signed-off-by: aszlig <aszlig@nix.build>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a few tests based on some assumptions based on properties of roman numerals, for example that in subtractive notation we should never have more than three of the same consecutive characters.
Right now these tests are very basic because in order to get more exhaustive we need to have access to some kind of mapping between roman digits and its values. Our current tests only cover properties that hold true without knowing more intrinsic details.
At the moment, the
encode_decode_symmetrytest fails with: