luhn: Update example and add more tests#253
Conversation
|
Thanks for the contribution! For most of our exercises we use the canonical test data defined in the x-common repo. If we're going to improve Luhn's test suite, we should start by changing the canonical test file. Then all the tracks can implement the improved tests. |
|
exercism/problem-specifications#522 has now landed, I've updated this, PTAL. |
| @@ -1,5 +1,6 @@ | |||
| pub fn is_valid(candidate: &str) -> bool { | |||
| if candidate.chars().any(|c| c.is_alphabetic()) || candidate.chars().count() == 1 { | |||
| if input.chars().filter(|c| c.is_digit(10)).take(2).count() <= 1 || | |||
There was a problem hiding this comment.
you seem to have intended to change the input's name to input, but didn't make the same change in the argument. I believe if you were to do that, the tests could pass.
There was a problem hiding this comment.
My bad, copying from my solution (which used input) to example, I forgot to change the name back. Hopefully fixed.
petertseng
left a comment
petertseng
left a comment
There was a problem hiding this comment.
righto, think we are doing well. I think just one small change and we can get this to compile and then merge it?
The luhn test suite didn't cover a lot of cases, including: - Strings of zeros - Non-alphabetic characters - SINs which are only valid if you remember to reverse the string - A 9 in a doubled position (9 * 2 - 9 should equal 9 not 0) - " 0" (invalid as there's only 1 digit) This adds those tests and updates the example to properly reject symbols.
petertseng
left a comment
petertseng
left a comment
There was a problem hiding this comment.
OK, seems faithful to x-common. There is a chance it will change due to exercism/problem-specifications#523, but until then let's get these in front of students, eh?
…#253) Closes exercism#253 * Create parallel-letter-frequency.md Initial list with concept level abstraction of common elements used for solving `parallel-letter-frequency`. * Update with specific concurrency concepts separate To-do: implement an example for each concurrency concept
…#253) Closes exercism#253 * Create parallel-letter-frequency.md Initial list with concept level abstraction of common elements used for solving `parallel-letter-frequency`. * Update with specific concurrency concepts separate To-do: implement an example for each concurrency concept
Closes exercism#253 * Create parallel-letter-frequency.md Initial list with concept level abstraction of common elements used for solving `parallel-letter-frequency`. * Update with specific concurrency concepts separate To-do: implement an example for each concurrency concept
Closes exercism#253 * Create parallel-letter-frequency.md Initial list with concept level abstraction of common elements used for solving `parallel-letter-frequency`. * Update with specific concurrency concepts separate To-do: implement an example for each concurrency concept
3 participants
The luhn test suite didn't cover a lot of cases, including:
This adds those tests and updates the example to properly reject symbols (and check that the sum isn't zero).
Let me know if I'm doing something wrong!
EDIT: FYI I was previously using
!c.is_whitespace()rather thanc != ' '. I switched because technically other whitespace like\tand\nisn't allowed by the letter of the rules.