atbash_cipher_test: Add 2 tests; leap: change all tests #406
Conversation
This file contains 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
Add `test_decode_number` as there is a test that encode number. Check this _iteration_[1] for a solution that pass the tests without checking for alphanumeric in decode. Add `test_encode_decode`. Having done this in the first place, would had highlighted the need of checking for alphanumeric in decoding. [1] http://exercism.io/submissions/c7c9448e6a674dd6a11ea35dc32e29a4
Predicate functions shouldn't convert its implementation based on
logic expressions (combinations of `not`, `and` and `or`).
I.e. The following should be correct:
def is_NOT_divisible_by_4(n):
return n % 4
assert is_NOT_divisible_by_4(10)
But as the TestCases for this exercise are using `assertIs(..., True)
instead of `assertTrue(...)`, implementations cointing on the numbers
behaviour on boolean contexts will Fail.
What this mean, is that for the given definition of
`is_NOT_divisible_by_4`, the folloging assertion will Fail:
assert is_NOT_divisible_by_4(10) == True
I think it is meaningless to check for `True` as it is naive to write:
if predicate(x) == True:
do_something(x)
Instead of:
if predicate(x):
do_something(x)
jgomo3
changed the title
|
Great changes! Would it be possible to split them, so that we have one PR per exercise? |
|
Sure. What do you suggest? |
|
I don't know an easier way than to create two new branches/prs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add
test_decode_numberas there is a test that encode number. Check this iteration[1] for a solution that pass the tests without checking for alphanumeric in decode.Add
test_encode_decode. Having done this in the first place, would had highlighted the need of checking for alphanumeric in decoding.[1] http://exercism.io/submissions/c7c9448e6a674dd6a11ea35dc32e29a4