Skip to content

Conversation

@stkent
Copy link
Contributor

@stkent stkent commented Feb 12, 2017

This updates the luhn examples to operate on even-length strings. This reinforces the idea that the numbers to be doubled are counted from the right, not from the left. (The previous odd-length example string did not emphasize this distinction.)

@stkent
Copy link
Contributor Author

stkent commented Feb 12, 2017

Related to #547


```
_4_ 4_4 _8_
4_3_ 1_8_ 0_4_ 6_6_
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The key change.


```
7+2+5+3+2+2+5+2+5+3+1+2+0+5+3+9 = 57
7+2+5+3+2+2+6+2+5+3+1+2+0+5+3+9 = 57
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed a typo in this sum!

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

I don't disagree with any change made herein, though now I claim that the commit message is inaccurate. The update to fix a typo is good but it has nothing to do with manipulating from the right, so it would fit in its own commit.

I pondered briefly whether it would be useful to keep some odd-length example in for the purpose of telling the student "double every second from the right doesn't mean you can just start at the leftmost digit" because that doesn't work for odd-length strings. But probably best left to the tests.

@stkent
Copy link
Contributor Author

stkent commented Feb 12, 2017

@petertseng commit split into two!

@stkent
Copy link
Contributor Author

stkent commented Feb 12, 2017

I agree on leaving the odd length examples to the tests; seems like the right place to discover that distinction if the (now multiple, consistent) clues from the description weren't enough.

@ErikSchierboom
Copy link
Member

LGTM!

@stkent
Copy link
Contributor Author

stkent commented Feb 14, 2017

Is there a "minimum number of approvals protocol" within this repo? I'm a little unclear as to when a PR is considered good to merge. Should I be merging this myself now that it has received 2 separate thumbs up? Thanks 🤓

Copy link
Contributor

@rbasso rbasso left a comment

Choose a reason for hiding this comment

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

If there is a minimum number of approvals, it is not bigger then 3! 😄

Is there a "minimum number of approvals protocol" within this repo? I'm a little unclear as to when a PR is considered good to merge. Should I be merging this myself now that it has received 2 separate thumbs up? Thanks 🤓

@stkent stkent merged commit 1dfd8e0 into master Feb 15, 2017
@stkent stkent deleted the luhn_examples branch February 15, 2017 03:20
emcoding pushed a commit that referenced this pull request Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants