Re: [Pharo-users] mentor question 4
This sounds very much like the Luhn test task at RosettaCode. https://rosettacode.org/wiki/Luhn_test_of_credit_card_numbers except that there it is described as working on the digits of an integer. (1) There are two approaches to traversing a sequence in reverse. (A) Reverse the sequence, then traverse the copy forward. aString reverse do: [:each | ...] (B) Just traverse the sequence in reverse aString reverseDo: [:each | ...] My taste is for the second. (2) There are two approaches to deleting spaces. (A) Make a copy of the string without spaces. x := aString reject: [:each | each = Character space]. x do: ... (B) Ignore spaces as you go: (i) aString do: [:each | each = Character space ifFalse: [...]] (ii) aString select: [:each | each ~= Character space] thenDo: [:each | ...] Combining (1A) and (2A) you get very obvious code: (aString reject: [:each | each = Character space]) reverse do: [:digit } ...] Combining (1B) and (2Bi) you get more efficient code: aString reverseDo: [:digit | digit = Character space ifFalse: [ ...]] By the way, let's start by checking that the character in the string *are* digits or spaces: (aString allSatisfy: [:each | each isDigit or: [each = Character s[ace]]) ifFalse: [^false], (3) There are two approaches to doubling the even digits. (A) Make a new string that starts as a copy and change every second digit from the right. (B) Simply *act* as if this has been done; keep track of whether the current digit position is even or odd and multiply by 1 or 2 as appropriate. nextIsOdd := true. aString reverseDo: [:digit | digit = Character space ifFalse: [ nextIsOdd ifTrue: [oddSum := ...] ifFalse: [evenSum := ...]. nextIsOdd := nextIsOdd not]]. I *like* code that traverses a data structure exactly once and allocates no intermediate garbage, so I'd be making (B) choices. But I am not at all sure that it is right for *you* at this stage. Your goal is to get practice in making code that is obviously correct and lends itself to testing. I think in your case it makes more sense to make the (A) choices. You can write some code that reverses a string (using the built-in method) and test that it does what you expect. You can write some code that checks whether a string contains only digits and spaces, and test that. You can write some code that returns a space-less copy, and test that. You can write some code that returns the even (odd) elements of a sequence, and test those. SequenceableCollection>> withIndexSelect: aBlock |index| index := 0. ^self select: [:each | aBlock value: each value: (index := index + 1)] evenElements ^self withIndexSelect: [:each :index | index even] oddElements ^self withIndexSelect: [:each :index | index odd] The easiest way to convert a string to numbers and add up the numbers is to use #detectSum, as in aString oddElements detectSum: [:char | char digitValue] How do you find #digitValue? By looking in Character. #detectSum:? Collection enumeration methods. Using the (A) approach will give you lots of little methods, which you can comment and above all TEST, so that each mistake will be in just one small method. This is a typical functional programming "lots of little functions" approach. On Thu, 30 Apr 2020 at 18:33, Roelof Wobben via Pharo-users <pharo-users@lists.pharo.org> wrote:
Hello,
I hope I can discuss my approch to this problem :
Given a number determine whether or not it is valid per the Luhn formula.
The 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.
The task is to check if a given string is valid.
Validating a Number
Strings of length 1 or less are not valid. Spaces are allowed in the input, but they should be stripped before checking. All other non-digit characters are disallowed.
Example 1: valid credit card number
4539 1488 0343 6467
The first step of the Luhn algorithm is to double every second digit, starting from the right. We will be doubling
4_3_ 1_8_ 0_4_ 6_6_
If doubling the number results in a number greater than 9 then subtract 9 from the product. The results of our doubling:
8569 2478 0383 3437
Then sum all of the digits:
8+5+6+9+2+4+7+8+0+3+8+3+3+4+3+7 = 80
If the sum is evenly divisible by 10, then the number is valid. This number is valid!
my idea was to do these steps
1) reverse the input.
2) use this to double every second digit and calculate the sum of all the numbers :
checkNumber := (collection reverse selectwith index: [:item :index | (index % 2 == 0) . IfTrue: [item *2]] ) sumNumbers
3) check if its a valid number by doing this :
^ (checkNumber % 10 == 0)
is this a good game plan or has it flaws or can I do it better ?
Regards,
Roelof
Op 30-4-2020 om 16:06 schreef Richard O'Keefe:
This sounds very much like the Luhn test task at RosettaCode. https://rosettacode.org/wiki/Luhn_test_of_credit_card_numbers except that there it is described as working on the digits of an integer.
(1) There are two approaches to traversing a sequence in reverse. (A) Reverse the sequence, then traverse the copy forward. aString reverse do: [:each | ...] (B) Just traverse the sequence in reverse aString reverseDo: [:each | ...] My taste is for the second.
(2) There are two approaches to deleting spaces. (A) Make a copy of the string without spaces. x := aString reject: [:each | each = Character space]. x do: ... (B) Ignore spaces as you go: (i) aString do: [:each | each = Character space ifFalse: [...]] (ii) aString select: [:each | each ~= Character space] thenDo: [:each | ...]
Combining (1A) and (2A) you get very obvious code: (aString reject: [:each | each = Character space]) reverse do: [:digit } ...] Combining (1B) and (2Bi) you get more efficient code: aString reverseDo: [:digit | digit = Character space ifFalse: [ ...]]
By the way, let's start by checking that the character in the string *are* digits or spaces: (aString allSatisfy: [:each | each isDigit or: [each = Character s[ace]]) ifFalse: [^false],
(3) There are two approaches to doubling the even digits. (A) Make a new string that starts as a copy and change every second digit from the right. (B) Simply *act* as if this has been done; keep track of whether the current digit position is even or odd and multiply by 1 or 2 as appropriate. nextIsOdd := true. aString reverseDo: [:digit | digit = Character space ifFalse: [ nextIsOdd ifTrue: [oddSum := ...] ifFalse: [evenSum := ...]. nextIsOdd := nextIsOdd not]].
I *like* code that traverses a data structure exactly once and allocates no intermediate garbage, so I'd be making (B) choices.
For me , I use this to practice solving problems and doing the "right" steps. So I love it , that so many people share there way of solving it. I can learn a lot from it Expecially when they explain there thinking process so detailed. I like this code also a lot. Am I correct for testing if it is a valid string by doing this ^ (oddSum + evenSum) dividedBy: 10 Roelof
Op 30-4-2020 om 16:16 schreef Roelof Wobben:
nextIsOdd := true.     aString reverseDo: [:digit |         digit = Character space ifFalse: [         nextIsOdd             ifTrue: [oddSum := ...]             ifFalse: [evenSum := ...].         nextIsOdd := nextIsOdd not]].
hmm, Still no luck with this code : cardNumber := '4539 1488 0343 6467'. oddSum := 0. evenSum := 0. nextIsOdd := true.     cardNumber reverseDo: [:digit |         digit = Character space ifFalse: [         nextIsOdd             ifTrue: [oddSum := oddSum + digit asInteger ]             ifFalse: [evenSum := ((digit asInteger * 2) > 9)    ifTrue: [evenSum + ((digit asInteger * 2) - 9) ]    ifFalse: [ evenSum + (digit asInteger * 2) ]].           nextIsOdd := nextIsOdd not]]. ^ oddSum + evenSum the answer schould be 57 where I get 1157 So some debugging to do
and also not with this one : cardNumber := '4539 1488 0343 6467'. oddSum := 0. evenSum := 0. nextIsOdd := false.     cardNumber reverseDo: [:digit |         digit = Character space ifFalse: [         nextIsOdd             ifFalse: [oddSum := oddSum + (digit asString asInteger ) ]             ifTrue: [(((digit asString asInteger) * 2) > 9)    ifTrue: [evenSum := evenSum + ((digit asString asInteger) * 2) - 9 ]    ifFalse: [ evenSum := evenSum + (digit asString asInteger) * 2 ]].            nextIsOdd := nextIsOdd not]]. ^ evenSum Op 30-4-2020 om 18:30 schreef Roelof Wobben:
Op 30-4-2020 om 16:16 schreef Roelof Wobben:
nextIsOdd := true.     aString reverseDo: [:digit |         digit = Character space ifFalse: [         nextIsOdd             ifTrue: [oddSum := ...]             ifFalse: [evenSum := ...].         nextIsOdd := nextIsOdd not]].
hmm,
Still no luck with this code :
cardNumber := '4539 1488 0343 6467'. oddSum := 0. evenSum := 0. nextIsOdd := true.     cardNumber reverseDo: [:digit |         digit = Character space ifFalse: [         nextIsOdd             ifTrue: [oddSum := oddSum + digit asInteger ]             ifFalse: [evenSum := ((digit asInteger * 2) > 9)    ifTrue: [evenSum + ((digit asInteger * 2) - 9) ]    ifFalse: [ evenSum + (digit asInteger * 2) ]].           nextIsOdd := nextIsOdd not]]. ^ oddSum + evenSum
the answer schould be 57 where I get 1157
So some debugging to do
(oddSum + evenSum) dividedBy: 10 You previously had _ isDivisibleBy: 10 which certainly works. Squeak, Pharo, and ST/X have #isDivisibleBy: VisualWorks. Dolphin, and GNU Smalltalk do not. Here's the code from Number.st in ST/X. isDivisibleBy:aNumber "return true, if the receiver can be divided by the argument, aNumber without a remainder. Notice, that the result is only worth trusting, if the receiver is an integer." aNumber = 0 ifTrue: [^ false]. aNumber isInteger ifFalse: [^ false]. ^ (self \\ aNumber) = 0 The comment is wrong: the question makes sense for any combination of exact numbers. When, as in this case, aNumber is a literal integer, all #isDivisibleBy: really adds is overhead. (oddSum + evenSum) \\ 10 = 0 is quite clear, and completely portable. On Fri, 1 May 2020 at 02:16, Roelof Wobben <r.wobben@home.nl> wrote:
Op 30-4-2020 om 16:06 schreef Richard O'Keefe:
This sounds very much like the Luhn test task at RosettaCode. https://rosettacode.org/wiki/Luhn_test_of_credit_card_numbers except that there it is described as working on the digits of an integer.
(1) There are two approaches to traversing a sequence in reverse. (A) Reverse the sequence, then traverse the copy forward. aString reverse do: [:each | ...] (B) Just traverse the sequence in reverse aString reverseDo: [:each | ...] My taste is for the second.
(2) There are two approaches to deleting spaces. (A) Make a copy of the string without spaces. x := aString reject: [:each | each = Character space]. x do: ... (B) Ignore spaces as you go: (i) aString do: [:each | each = Character space ifFalse: [...]] (ii) aString select: [:each | each ~= Character space] thenDo: [:each | ...]
Combining (1A) and (2A) you get very obvious code: (aString reject: [:each | each = Character space]) reverse do: [:digit } ...] Combining (1B) and (2Bi) you get more efficient code: aString reverseDo: [:digit | digit = Character space ifFalse: [ ...]]
By the way, let's start by checking that the character in the string *are* digits or spaces: (aString allSatisfy: [:each | each isDigit or: [each = Character s[ace]]) ifFalse: [^false],
(3) There are two approaches to doubling the even digits. (A) Make a new string that starts as a copy and change every second digit from the right. (B) Simply *act* as if this has been done; keep track of whether the current digit position is even or odd and multiply by 1 or 2 as appropriate. nextIsOdd := true. aString reverseDo: [:digit | digit = Character space ifFalse: [ nextIsOdd ifTrue: [oddSum := ...] ifFalse: [evenSum := ...]. nextIsOdd := nextIsOdd not]].
I *like* code that traverses a data structure exactly once and allocates no intermediate garbage, so I'd be making (B) choices.
For me , I use this to practice solving problems and doing the "right" steps. So I love it , that so many people share there way of solving it. I can learn a lot from it Expecially when they explain there thinking process so detailed.
I like this code also a lot. Am I correct for testing if it is a valid string by doing this ^ (oddSum + evenSum) dividedBy: 10
Roelof
Op 1-5-2020 om 02:51 schreef Richard O'Keefe:
(oddSum + evenSum) dividedBy: 10
You previously had _ isDivisibleBy: 10 which certainly works.
Squeak, Pharo, and ST/X have #isDivisibleBy: VisualWorks. Dolphin, and GNU Smalltalk do not.
Here's the code from Number.st in ST/X. isDivisibleBy:aNumber "return true, if the receiver can be divided by the argument, aNumber without a remainder. Notice, that the result is only worth trusting, if the receiver is an integer."
aNumber = 0 ifTrue: [^ false]. aNumber isInteger ifFalse: [^ false]. ^ (self \\ aNumber) = 0
The comment is wrong: the question makes sense for any combination of exact numbers. When, as in this case, aNumber is a literal integer, all #isDivisibleBy: really adds is overhead.
(oddSum + evenSum) \\ 10 = 0
is quite clear, and completely portable.
On Fri, 1 May 2020 at 02:16, Roelof Wobben <r.wobben@home.nl> wrote:
Op 30-4-2020 om 16:06 schreef Richard O'Keefe:
This sounds very much like the Luhn test task at RosettaCode. https://rosettacode.org/wiki/Luhn_test_of_credit_card_numbers except that there it is described as working on the digits of an integer.
(1) There are two approaches to traversing a sequence in reverse. (A) Reverse the sequence, then traverse the copy forward. aString reverse do: [:each | ...] (B) Just traverse the sequence in reverse aString reverseDo: [:each | ...] My taste is for the second.
(2) There are two approaches to deleting spaces. (A) Make a copy of the string without spaces. x := aString reject: [:each | each = Character space]. x do: ... (B) Ignore spaces as you go: (i) aString do: [:each | each = Character space ifFalse: [...]] (ii) aString select: [:each | each ~= Character space] thenDo: [:each | ...]
Combining (1A) and (2A) you get very obvious code: (aString reject: [:each | each = Character space]) reverse do: [:digit } ...] Combining (1B) and (2Bi) you get more efficient code: aString reverseDo: [:digit | digit = Character space ifFalse: [ ...]]
By the way, let's start by checking that the character in the string *are* digits or spaces: (aString allSatisfy: [:each | each isDigit or: [each = Character s[ace]]) ifFalse: [^false],
(3) There are two approaches to doubling the even digits. (A) Make a new string that starts as a copy and change every second digit from the right. (B) Simply *act* as if this has been done; keep track of whether the current digit position is even or odd and multiply by 1 or 2 as appropriate. nextIsOdd := true. aString reverseDo: [:digit | digit = Character space ifFalse: [ nextIsOdd ifTrue: [oddSum := ...] ifFalse: [evenSum := ...]. nextIsOdd := nextIsOdd not]].
I *like* code that traverses a data structure exactly once and allocates no intermediate garbage, so I'd be making (B) choices.
For me , I use this to practice solving problems and doing the "right" steps. So I love it , that so many people share there way of solving it. I can learn a lot from it Expecially when they explain there thinking process so detailed.
I like this code also a lot. Am I correct for testing if it is a valid string by doing this ^ (oddSum + evenSum) dividedBy: 10
Roelof
oke, so this is better cardNumber := '8273 1232 7352 0569'. oddSum := 0. evenSum := 0. nextIsOdd := false.     cardNumber reverseDo: [:character |         digit := character digitValue.         character = Character space ifFalse: [         nextIsOdd             ifFalse: [oddSum := oddSum + digit ]             ifTrue: [(digit >= 5 )    ifTrue: [evenSum := evenSum + (digit * 2) - 9 ]    ifFalse: [ evenSum := evenSum + (digit * 2) ]].            nextIsOdd := nextIsOdd not]]. ^ evenSum + oddSum // 10 == 0. where I could even make a seperate method of the ifTrue branch when the digit is greater then 5.
Op 1-5-2020 om 08:35 schreef Roelof Wobben:
Op 1-5-2020 om 02:51 schreef Richard O'Keefe:
(oddSum + evenSum) dividedBy: 10
You previously had _ isDivisibleBy: 10 which certainly works.
Squeak, Pharo, and ST/X have #isDivisibleBy: VisualWorks. Dolphin, and GNU Smalltalk do not.
Here's the code from Number.st in ST/X. isDivisibleBy:aNumber     "return true, if the receiver can be divided by the argument, aNumber without a remainder.      Notice, that the result is only worth trusting, if the receiver is an integer."
    aNumber = 0 ifTrue: [^ false].     aNumber isInteger ifFalse: [^ false].     ^ (self \\ aNumber) = 0
The comment is wrong: the question makes sense for any combination of exact numbers. When, as in this case, aNumber is a literal integer, all #isDivisibleBy: really adds is overhead.
(oddSum + evenSum) \\ 10 = 0
is quite clear, and completely portable.
On Fri, 1 May 2020 at 02:16, Roelof Wobben <r.wobben@home.nl> wrote:
Op 30-4-2020 om 16:06 schreef Richard O'Keefe:
This sounds very much like the Luhn test task at RosettaCode. https://rosettacode.org/wiki/Luhn_test_of_credit_card_numbers except that there it is described as working on the digits of an integer.
(1) There are two approaches to traversing a sequence in reverse.      (A) Reverse the sequence, then traverse the copy forward.          aString reverse do: [:each | ...]      (B) Just traverse the sequence in reverse          aString reverseDo: [:each | ...]      My taste is for the second.
(2) There are two approaches to deleting spaces. Â Â Â Â Â (A) Make a copy of the string without spaces. Â Â Â Â Â Â Â Â Â x := aString reject: [:each | each = Character space]. Â Â Â Â Â Â Â Â Â x do: ... Â Â Â Â Â (B) Ignore spaces as you go: Â Â Â Â Â Â Â Â Â (i) aString do: [:each | each = Character space ifFalse: [...]] Â Â Â Â Â Â Â Â Â (ii) aString select: [:each | each ~= Character space] thenDo: [:each | ...]
Combining (1A) and (2A) you get very obvious code: Â Â Â Â Â (aString reject: [:each | each = Character space]) reverse do: [:digit } ...] Combining (1B) and (2Bi) you get more efficient code: Â Â Â Â Â aString reverseDo: [:digit | Â Â Â Â Â Â Â Â Â digit = Character space ifFalse: [ ...]]
By the way, let's start by checking that the character in the string *are* digits or spaces: Â Â Â Â Â (aString allSatisfy: [:each | each isDigit or: [each = Character s[ace]]) Â Â Â Â Â Â Â Â Â ifFalse: [^false],
(3) There are two approaches to doubling the even digits.      (A) Make a new string that starts as a copy and change every second           digit from the right.      (B) Simply *act* as if this has been done; keep track of whether the          current digit position is even or odd and multiply by 1 or 2 as          appropriate.      nextIsOdd := true.      aString reverseDo: [:digit |          digit = Character space ifFalse: [          nextIsOdd              ifTrue: [oddSum := ...]              ifFalse: [evenSum := ...].          nextIsOdd := nextIsOdd not]].
I *like* code that traverses a data structure exactly once and allocates no intermediate garbage, so I'd be making (B) choices.
For me , I use this to practice solving problems and doing the "right" steps. So I love it , that so many people share there way of solving it. I can learn a lot from it Expecially when they explain there thinking process so detailed.
I like this code also a lot. Am I correct for testing if it is a valid string by doing this ^ (oddSum + evenSum) dividedBy: 10
Roelof
oke,
so this is better
cardNumber := '8273 1232 7352 0569'. oddSum := 0. evenSum := 0. nextIsOdd := false.     cardNumber reverseDo: [:character |         digit := character digitValue.         character = Character space ifFalse: [         nextIsOdd             ifFalse: [oddSum := oddSum + digit ]             ifTrue: [(digit >= 5 )    ifTrue: [evenSum := evenSum + (digit * 2) - 9 ]    ifFalse: [ evenSum := evenSum + (digit * 2) ]].            nextIsOdd := nextIsOdd not]]. ^ evenSum + oddSum // 10 == 0.
where I could even make a seperate method of the ifTrue branch when the digit is greater then 5.
nobody who can say if this is a good solution ? Roelof
participants (2)
-
Richard O'Keefe -
Roelof Wobben