Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luhn: tweak canonical tests #547

Merged
merged 2 commits into from
Feb 21, 2017
Merged

luhn: tweak canonical tests #547

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aff180a
luhn: tweak canonical tests
stkent Feb 12, 2017
9d5b690
luhn: further adjustments to canonical test suite
stkent Feb 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions exercises/luhn/canonical-data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"expected": false
},
{
"description": "simple valid sin",
"description": "simple valid SIN",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalized sin -> SIN throughout.

"input": " 5 9 ",
"expected": true
},
Expand All @@ -20,6 +20,11 @@
"input": "046 454 286",
"expected": true
},
{
"description": "another valid Canadian SIN",
"input": "055 444 285",
"expected": true
},
Copy link
Contributor Author

@stkent stkent Feb 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PR that added this test, the following reasoning was given:

Part of it was the contrast with 055-444-285, and part of it was the thought that having a couple of different longer valid numbers might help someone who was struggling to work out how the algorithm needed to be implemented.

I moved it earlier in the suite so that it is more likely encountered by someone who is still trying to work out the algorithm, and updated the language in the test cases that contrast with this one to make that comparison more clear.

{
"description": "invalid Canadian SIN",
"input": "046 454 287",
Expand All @@ -31,17 +36,17 @@
"expected": false
},
{
"description": "valid strings with a non-digit added become invalid",
"description": "valid strings with a non-digit included become invalid",
"input": "046a 454 286",
"expected": false
},
{
"description": "punctuation is not allowed",
"description": "valid strings with punctuation included become invalid",
"input": "055-444-285",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare to valid case 055 444 285.

"expected": false
},
{
"description": "symbols are not allowed",
"description": "valid strings with symbols included become invalid",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare to valid case 055 444 285.

"input": "055£ 444$ 285",
"expected": false
},
Expand All @@ -51,17 +56,12 @@
"expected": false
},
{
"description": "lots of zeros are valid",
"input": " 00000",
"expected": true
},
{
"description": "another valid sin",
"input": "055 444 285",
"description": "more than a single zero is valid",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to state the limitation here more clearly, and moved the space character to indicate that a "chain" of zeros is not required.

"input": "00 000",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "0 0000" would be even better here?

"expected": true
},
{
"description": "nine doubled is nine",
"description": "input digit 9 is correctly converted to output digit 9",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nine doubled is not nine; tried to make this a little more explicit!

"input": "091",
"expected": true
}
Expand Down