From fb10db76a25b6a05f87d4f89461974d500f0e441 Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Wed, 21 Dec 2016 15:57:08 -0600 Subject: [PATCH 1/6] Update Luhn tests and description I'm addressing a few different things here: 1. The tests exposed functionality that is not needed The `addends` and `check_digit` methods aren't needed by clients of this code, and I didn't see that students got any benefit from explicitly implementing them. The presence of these tests pushed students towards a particular implementation, which we [try to avoid](https://github.com/exercism/x-common/blob/master/CONTRIBUTING.md#improving-consistency-by-extracting-shared-test-data) > The test cases should not require people to follow one very specific way to solve the problem, e.g. the tests should avoid checking for helper functions that might not be present in other implementations. To fix this I've removed tests of `addends` and `check_digit`. 2. The tests were duplicative. There's not a way (that I could see, at least) of implementing Luhn in tiny steps. It's either working or its not. Once you have a couple valid tests and a couple invalid tests your are pretty much covered. (note: I guess you could test the cases where Luhn can be wrong, but that seemed outside the scope of this exercise.) Again, from the guide > All tests should be essential to the problem. Ensure people can get the first test passing easily. Then each following test should ideally add a little functionality. People should be able to solve the problem a little at a time. Avoid test cases that don't introduce any new aspect and would already pass anyway. To fix this I've removed most duplicative tests that would pass as soon as the student got a working Luhn implementation. 3. The examples and the description didn't match Luhn can be hard to grasp. At least it can be for me. What helps me is to have explicit, real-world examples. To help with this I've updated the description.md file to have examples of credit cards and Canadian SINs, two places where Luhn is used in the real world. To re-enforce the descriptions, I've used the same examples in the tests. So students can see step-by-step breakdowns of how to get their tests passing. Note: I'm still on the fence about the `generate_check_digit` method. It seems to be to be outside the scope of this exercise, but I also think implementing it is interesting and useful. I could go either way on keeping it or removing it. --- exercises/luhn/canonical-data.json | 97 +++++------------ exercises/luhn/description.md | 163 ++++++++++++++++++++--------- 2 files changed, 140 insertions(+), 120 deletions(-) diff --git a/exercises/luhn/canonical-data.json b/exercises/luhn/canonical-data.json index 6136287523..b087d233f5 100644 --- a/exercises/luhn/canonical-data.json +++ b/exercises/luhn/canonical-data.json @@ -1,70 +1,31 @@ { - "#": [ - "The only note about this exercise is to make sure that the methods used", - "to check if a number is valid and to compute its checksum DO NOT modify", - "the value of the internal attributes. ", - "I say this because I've seen for the Python testcases that it needs a last", - "test to call the validation method twice to assure this. ", - "Simply assure that the two methods are const methods (to use a C++ notation)", - "using the tools available in your language. ", - "Here is a pretty universal solution for the validation method: ", - "`return (0 === (this.checksum % 10)) ? true : false;`" - ], - "cases": [ - { - "description": "check digit", - "input": 34567, - "expected": 7 - }, - { - "description": "check digit with input ending in zero", - "input": 91370, - "expected": 0 - }, - { - "description": "check addends", - "input": 12121, - "expected": [1, 4, 1, 4, 1] - }, - { - "description": "check too large addends", - "input": 8631, - "expected": [7, 6, 6, 1] - }, - { - "description": "checksum", - "input": 4913, - "expected": 22 - }, - { - "description": "checksum of larger number", - "input": 201773, - "expected": 21 - }, - { - "description": "check invalid number", - "input": 738, - "expected": false - }, - { - "description": "check valid number", - "input": 8739567, - "expected": true - }, - { - "description": "create valid number", - "input": 123, - "expected": 1230 - }, - { - "description": "create larger valid number", - "input": 873956, - "expected": 8739567 - }, - { - "description": "create even larger valid number", - "input": 837263756, - "expected": 8372637564 - } - ] + "valid": [ + { + "description": "valid Canadian SIN", + "input": "046 454 286", + "expected": true + }, + { + "description": "invalid Canadian SIN", + "input": "046 454 287", + "expected": false + }, + { + "description": "invalid credit card", + "input": "8273 1232 7352 0569", + "expected": false + } + ], + "generate_check_digit": [ + { + "description": "get valid check digit for Canadian SIN", + "input": "046 454 28", + "expected": 6 + }, + { + "description": "get valid check digit for credit account", + "input": "8273 1232 7352 056", + "expected": 2 + } + ] } diff --git a/exercises/luhn/description.md b/exercises/luhn/description.md index 2f9da2b1a1..d22c7efe76 100644 --- a/exercises/luhn/description.md +++ b/exercises/luhn/description.md @@ -1,53 +1,112 @@ -The Luhn formula is a simple checksum formula used to validate a variety -of identification numbers, such as credit card numbers and Canadian -Social Insurance Numbers. - -The formula verifies a number against its included check digit, which is -usually appended to a partial number to generate the full number. This -number must pass the following test: - -- Counting from rightmost digit (which is the check digit) and moving - left, double the value of every second digit. -- For any digits that thus become 10 or more, subtract 9 from the - result. - - 1111 becomes 2121. - - 8763 becomes 7733 (from 2×6=12 → 12-9=3 and 2×8=16 → 16-9=7). -- Add all these digits together. - - 1111 becomes 2121 sums as 2+1+2+1 to give a check digit of 6. - - 8763 becomes 7733, and 7+7+3+3 is 20. - -If the total ends in 0 (put another way, if the total modulus 10 is -congruent to 0), then the number is valid according to the Luhn formula; -else it is not valid. So, 1111 is not valid (as shown above, it comes -out to 6), while 8763 is valid (as shown above, it comes out to 20). - -Write a program that, given a number - -- Can check if it is valid per the Luhn formula. This should treat, for - example, "2323 2005 7766 3554" as valid. -- Can return the checksum, or the remainder from using the Luhn method. -- Can add a check digit to make the number valid per the Luhn formula and - return the original number plus that digit. This should give "2323 2005 7766 - 3554" in response to "2323 2005 7766 355". - -## About Checksums - -A checksum has to do with error-detection. There are a number of different -ways in which a checksum could be calculated. - -When transmitting data, you might also send along a checksum that says how -many bytes of data are being sent. That means that when the data arrives on -the other side, you can count the bytes and compare it to the checksum. If -these are different, then the data has been garbled in transmission. - -In the Luhn problem the final digit acts as a sanity check for all the prior -digits. Running those prior digits through a particular algorithm should give -you that final digit. - -It doesn't actually tell you if it's a real credit card number, only that it's -a plausible one. It's the same thing with the bytes that get transmitted -- -you could have the right number of bytes and still have a garbled message. So -checksums are a simple sanity-check, not a real in-depth verification of the -authenticity of some data. It's often a cheap first pass, and can be used to -quickly discard obviously invalid things. +The [Luhn algorithm](https://en.wikipedia.org/wiki/Luhn_algorithm) is +a simple checksum formula used to validate a variety of identification +numbers, such as credit card numbers and Canadian Social Insurance +Numbers. +Validating a Number +------ + +As an example, here is a valid (but fictitious) Canadian Social Insurance +Number. + +``` +046 454 286 +``` + +The first step of the Luhn algorithm is to double every second digit, +starting from the right. We will be doubling + +``` +_4_ 4_4 _8_ +``` + +If doubling the number results in a number greater than 9 then subtract 9 +from the product. The results of our doubling: + +``` +086 858 276 +``` + +Then sum all of the digits + +``` +0+8+6+8+5+8+2+7+6 = 50 +``` + +If the sum is evenly divisible by 10, then the number is valid. This number is valid! + +An example of an invalid Canadian SIN where we've changed the final digit + +``` +046 454 287 +``` + +Double the second digits, starting from the right + +``` +086 858 276 +``` + +Sum the digits + +``` +0+8+6+8+5+8+2+7+7 = 51 +``` + +51 is not evenly divisible by 10, so this number is not valid. + +---- + +An example of an invalid credit card account + +``` +8273 1232 7352 0569 +``` + +Double the second digits, starting from the right + +``` +7253 2262 5312 0539 +``` + +Sum the digits + +``` +7+2+5+3+2+2+5+2+5+3+1+2+0+5+3+9 = 57 +``` + +57 is not evenly divisible by 10, so this number is not valid. + +Generating a Check Digit +---- + +The final number in these examples is called to a [check digit](https://en.wikipedia.org/wiki/Check_digit). + +``` + 046 454 28 6 +| account | |check digit| +``` + +``` +8273 1232 7352 056 9 +| account | |check digit| +``` + +Given an account number your code should be able to return a check digit +that can be appended to the account to generate a valid Luhn + +Generate a correct check digit for our Canadian SIN account number + +``` +Luhn.generate_check_digit("046 454 28") +#=> 6 +#=> 045 456 286 is valid +``` + +Generate a correct check digit for our credit card account + +``` +Luhn.generate_check_digit("8273 1232 7352 056") +#=> 2 +#=> 8273 1232 7352 0562 is valid +``` From 243d86b70476dbf62a34de7757f5a11a5d6478ef Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sat, 31 Dec 2016 14:03:50 -0600 Subject: [PATCH 2/6] Fix incorrect example --- exercises/luhn/description.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exercises/luhn/description.md b/exercises/luhn/description.md index d22c7efe76..a0758bf60d 100644 --- a/exercises/luhn/description.md +++ b/exercises/luhn/description.md @@ -44,7 +44,7 @@ An example of an invalid Canadian SIN where we've changed the final digit Double the second digits, starting from the right ``` -086 858 276 +086 858 277 ``` Sum the digits From f5be88255033b86bace7e2fc61327dc3ec3c13b6 Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sat, 31 Dec 2016 14:04:10 -0600 Subject: [PATCH 3/6] Test edge cases, remove unnecessary tests No one voiced a strong argument for keeping the Check Digit tests, so I'm removing the test and their section in the description. As recommended, I'm adding 3 tests - invalid string containing a non-digit - Single digit 0 (which can return 0 for `modulo 10`) is not valide - Non-zero single digit is invalid --- exercises/luhn/canonical-data.json | 23 +++++++++++--------- exercises/luhn/description.md | 34 ------------------------------ 2 files changed, 13 insertions(+), 44 deletions(-) diff --git a/exercises/luhn/canonical-data.json b/exercises/luhn/canonical-data.json index b087d233f5..f573907cfe 100644 --- a/exercises/luhn/canonical-data.json +++ b/exercises/luhn/canonical-data.json @@ -14,18 +14,21 @@ "description": "invalid credit card", "input": "8273 1232 7352 0569", "expected": false - } - ], - "generate_check_digit": [ + }, { - "description": "get valid check digit for Canadian SIN", - "input": "046 454 28", - "expected": 6 + "description": "single digit strings can not be valid", + "input": "1", + "expected": false }, { - "description": "get valid check digit for credit account", - "input": "8273 1232 7352 056", - "expected": 2 - } + "description": "A single zero is invalid", + "input": "0", + "expected": false + }, + { + "description": "strings that contain non-digits are not valid", + "input": "827a 1232 7352 0569", + "expected": false + }, ] } diff --git a/exercises/luhn/description.md b/exercises/luhn/description.md index a0758bf60d..b6885030be 100644 --- a/exercises/luhn/description.md +++ b/exercises/luhn/description.md @@ -76,37 +76,3 @@ Sum the digits ``` 57 is not evenly divisible by 10, so this number is not valid. - -Generating a Check Digit ----- - -The final number in these examples is called to a [check digit](https://en.wikipedia.org/wiki/Check_digit). - -``` - 046 454 28 6 -| account | |check digit| -``` - -``` -8273 1232 7352 056 9 -| account | |check digit| -``` - -Given an account number your code should be able to return a check digit -that can be appended to the account to generate a valid Luhn - -Generate a correct check digit for our Canadian SIN account number - -``` -Luhn.generate_check_digit("046 454 28") -#=> 6 -#=> 045 456 286 is valid -``` - -Generate a correct check digit for our credit card account - -``` -Luhn.generate_check_digit("8273 1232 7352 056") -#=> 2 -#=> 8273 1232 7352 0562 is valid -``` From 837265fa942d7b51154e5dc63cb5ac047ab23fe4 Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sat, 31 Dec 2016 14:08:13 -0600 Subject: [PATCH 4/6] Move two simplest tests to start of suite --- exercises/luhn/canonical-data.json | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/exercises/luhn/canonical-data.json b/exercises/luhn/canonical-data.json index f573907cfe..4752d5cf09 100644 --- a/exercises/luhn/canonical-data.json +++ b/exercises/luhn/canonical-data.json @@ -1,5 +1,15 @@ { "valid": [ + { + "description": "single digit strings can not be valid", + "input": "1", + "expected": false + }, + { + "description": "A single zero is invalid", + "input": "0", + "expected": false + }, { "description": "valid Canadian SIN", "input": "046 454 286", @@ -15,16 +25,6 @@ "input": "8273 1232 7352 0569", "expected": false }, - { - "description": "single digit strings can not be valid", - "input": "1", - "expected": false - }, - { - "description": "A single zero is invalid", - "input": "0", - "expected": false - }, { "description": "strings that contain non-digits are not valid", "input": "827a 1232 7352 0569", From 60c733c7f98fdf59b1172ba71e674dfac531f561 Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sun, 1 Jan 2017 08:25:24 -0600 Subject: [PATCH 5/6] Stating the goal of the exercise --- exercises/luhn/description.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exercises/luhn/description.md b/exercises/luhn/description.md index b6885030be..d22781a1d7 100644 --- a/exercises/luhn/description.md +++ b/exercises/luhn/description.md @@ -3,6 +3,8 @@ a simple checksum formula used to validate a variety of identification numbers, such as credit card numbers and Canadian Social Insurance Numbers. +The task is to write a function that checks if a given string is valid. + Validating a Number ------ From e801f39b3ff4b1f9c7a70d1aa0b19ea4c64f6eb6 Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sun, 1 Jan 2017 08:27:07 -0600 Subject: [PATCH 6/6] Fix invalid json --- exercises/luhn/canonical-data.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exercises/luhn/canonical-data.json b/exercises/luhn/canonical-data.json index 4752d5cf09..87e2c236a5 100644 --- a/exercises/luhn/canonical-data.json +++ b/exercises/luhn/canonical-data.json @@ -29,6 +29,6 @@ "description": "strings that contain non-digits are not valid", "input": "827a 1232 7352 0569", "expected": false - }, + } ] }