It's a good solution. On Sat, May 2, 2020, 00:52 Roelof Wobben via Pharo-users < pharo-users@lists.pharo.org> wrote:

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