On Sat, May 2, 2020 at 12:38 PM Roelof Wobben via Pharo-users < pharo-users@lists.pharo.org> wrote:
> Op 2-5-2020 om 21:09 schreef Roelof Wobben via Pharo-users: > > > but this seems to work : > > cardNumber := '4539 1488 0343 6467'. > cleanDigits := cardNumber copyWithout: Character space. > preWrapDigits := (cleanDigits asArray reverse collectWithIndex: [ :char > :idx | (idx even) ifTrue: [ 2 * char digitValue ] ifFalse:[char > digitValue]]). > wrappedDigits := preWrapDigits collect: [ :d | d > 9 ifFalse: [ d ] > ifTrue: [ d-9 ] ]. > ^ wrappedDigits sum isDivisibleBy: 10. > > Can the code be more improved ? > As a general guideline, the best code will correspond closely to the documented algorithm. In this latest example, you have two visible loop iterations plus to hidden loop iterations. It also includes object creation from both #asArray and #reverse, the two messages with the hidden loop iteration. For this exercise, the number of loops and object creations isn't relevant. However, you always want to understand when you are doing such things. They will have an impact if they are used inside a high frequency loop. Again, that's not the case with this exercise. It often is in real world applications. But again, you should not optimize for performance at the expense of clarity and readability until you need to. And when you do need to, you annotate the code to explain how the implementation corresponds to or differs from the more comprehensible implementation. (Comments should explain the why and/or the background rather than the what.) > Roelof > > >
