Maybe this is a naive question, but can you just split the task into the following two? 1. Check whether whether the string is syntactically an ISBN number. This can be done, e.g., using a regex. 2. Check the the check character. Calculate the check character from the (now to be known) syntactically valid string. ISBNValidator>>isValidISBN: aString ^(self isSyntacticallyValid: aString) and: [self isCheckCharacterValid: aString] Kind regards, Steffen Am .09.2020, 07:35 Uhr, schrieb Roelof Wobben via Pharo-users <pharo-users@lists.pharo.org>:

Nope, with your idea I cannot make this part work :

he ISBN-10 format is 9 digits (0 to 9) plus one check character (either a digit or an X only). In the case the check character is an X, this represents the value '10'. These may be communicated with or without hyphens, and can be checked for their validity by the following formula:

(x1 * 10 + x2 * 9 + x3 * 8 + x4 * 7 + x5 * 6 + x6 * 5 + x7 * 4 + x8 * 3 + x9 * 2 +

so I mean the calculation.

Roelof

Op 4-9-2020 om 06:45 schreef Roelof Wobben:

oke, then I could use your idea but then I have to make the code for calculating if its a valid number. and I wonder if the code will not be too big. I learned that it is good that a method does only 1 thing and this one seems to be doing more then 1 thing.

Roelof

Op 4-9-2020 om 05:24 schreef Richard O'Keefe:

What part of "return false if there are not exactly 10 characters

left after discarding dashes" fails to handle the empty string?

A test case for the empty string is is only valuable if the

empty string is NOT a special case.

On Wed, 2 Sep 2020 at 22:52, Roelof Wobben <r.wobben@home.nl> wrote:

Op 2-9-2020 om 12:38 schreef Richard O'Keefe:

There is simply no point in "taking the first nine numbers out".

And there shouldn't BE a test for the string being empty, anywhere.

'' '-' '---' and so on should all be handled the same way.

Oh well, what stops you doing

digits := aString select: [:each | each ~= $-].

digits size = 10 ifFalse: [^false].

lastDigit := digits la ost.

digits := digits copyFrom: 1 to: 9.

( (lastDigit = $X or: [lastDigit isDigit]) and: [

digits allSatisfy: #isDigit]

) ifFalse: [^false].

Now my code does not do this, but it is just 16 lines of code with

nothing that it would make sense to extract.

Nothing only that I could not think of this one for myself. If I do it the TDD way I come more on the way Im currently thinking

but does this case then be covered

test14_EmptyIsbn | result | result := isbnVerifierCalculator isValidIsbn: ''. self assert: result equals: false

and still I have to do the calcualation to see if it's valid. If I understand the code well I can use the digits variable ?

Roelof

No problem. I am not knowledgeable about isbn numbers. At which places may a dash occur? Kind regards, Steffen 07.09.2020 16:18:22 Roelof Wobben via Pharo-users <pharo-users@lists.pharo.org>:

Op 6-9-2020 om 10:07 schreef Steffen Märcker:

...

Maybe this is a naive question, but can you just split the task into the following two?

1. Check whether whether the string is syntactically an ISBN number. This can be done, e.g., using a regex.

2. Check the the check character. Calculate the check character from the (now to be known) syntactically valid string.

ISBNValidator>>isValidISBN: aString ^(self isSyntacticallyValid: aString) and: [self isCheckCharacterValid: aString]

Kind regards, Steffen nder if the code will not be too big. I learned that it is good

Sorry to respond not earlier but your respons seems to be in the spam folder of my provider.

I could do that but if very bad in regex so I do not know a regex which van validate 123456789 or 123-456-78-9

Roelof

Hi, after reading the link and some additional sources, it turns out that a valid ISBN-10 has either no separators or four blocks separated by either dashes or spaces: Group-Publisher-Title-CheckDigit Assuming Regex11 (and that I made no mistake), the following should do the trick: IsbnVarifier>>isSyntacticIsbn: aString "no groups" noGrouped := '\d{9}[0-9X]' asRegex. "groups separated by either dashes or spaces" dashes := '\d{1,7}-\d{1,7}-\d{1,7}-[0-9X]' spaces := '\d{1,7} \d{1,7} \d{1,7} [0-9X]' grouped := (dashed , '|' , spaces) asRegex. ^(aString matches: nonGrouped) or: [(aString matches: grouped) and: [aString size = 10 + 3]] Surely, you could cleverly compress the regex even further but that does not matter for this example. After checking the syntax, you can just iterate over the string and compute the check-digit on the fly. Kind regards, Steffen Am .09.2020, 18:19 Uhr, schrieb Roelof Wobben via Pharo-users <pharo-users@lists.pharo.org>:

See here for all the tests : https://github.com/exercism/pharo-smalltalk/blob/master/exercises/isbn-verif...

Roelof

There are two quite different questions. (1) Where may dashes occur in a real ISBN-10? (2) What does Exercism require in the specification and check in the test cases? For (1) the rules are Each ISBN consists of 5 elements with each section being separated by spaces or hyphens. Three of the five elements may be of varying length: - *Prefix element* – currently this can only be either 978 or 979. It is always 3 digits in length - *Registration group element* – this identifies the particular country, geographical region, or language area participating in the ISBN system. This element may be between 1 and 5 digits in length - *Registrant element* - this identifies the particular publisher or imprint. This may be up to 7 digits in length - *Publication element* – this identifies the particular edition and format of a specific title. This may be up to 6 digits in length - *Check digit* – this is always the final single digit that mathematically validates the rest of the number. It is calculated using a Modulus 10 system with alternate weights of 1 and 3. An ISBN-10 does not have the three-digit prefix. So we have [0-9]{1,5} -- prefix [0-9]{1,7} -- registrant [0-9]{1,6} -- publication [0-9X] -- check digit As an examplw, "Standard C++ IOStreams and Locales" by Langer & Kreft has ISBN-10 0-201-18395-1 ISBN-13 9780201183955 so I shall assume the separators are optional. /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ Of course the elements cannot all have their maximum length at the same time. In AWK I would write x = a_putative_ISBN_10 y = x gsub(/[- ]+/, "", y) if (x ~ /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ \ && y ~ /^[0-9]{9,9}[0-9X]$/ \ ) { x *might* be valid, we still need to check the checksum } For (2), there appear to be no restrictions on where dashes may occur or how many: "These may be communicated with or without hyphens". Exercism doesn't allow spaces. Regular expressions are elegant in their own way, BUT for this problem they are (a) excessive, (b) inefficient, and (c) insufficient. digit count := 0. check sum := 0. for each character c of the string if c is not a hyphen then if c is a digit then digit value := c's value as a digit else if c is X and digit count = 9 then digit value := 10 else return false. digit count := digit count + 1. if digit count > 10 then return false. check sum := (11 - digit count) * digit value + check sum. return check sum mod 11 is zero. Part of the insight here is "don't DO it, just PRETEND you did." That is, instead of copying the string without the hyphens, just ignore the hyphens as they turn up. Another part is "if you are only going to use it once, don't store it." That is, we need a digit's value just once, in the update to check sum, so we should compute it just before we need it, not store it. Now the pseudo-code above is classic sequential imperative coding. Classic functional coding does something like let no_dashes = filter (/= '-') (explode string) in length no_dashes = 10 and let check = last no_dashes in (is_digit check or check = 'X') and all is_digit (take 9 no_dashes) and let xval c = if x = 'X' then 10 else digit_value c in dot (map xval no_dashes) [10,9..1]) mod 11 = 0 This pseudo-code translates nicely to Smalltalk too. You might want to add SequenceableCollection>> with: other inject: initial into: aBlock |r| r := initial. self with: other do: [:x :y | r := aBlock value: r value: x value: y]. ^r dot: other ^self with: other inject: 0 into: [:acc :x :y | x*y + acc] (These methods are so obvious that it would be absurd to claim any intellectual property rights to them.) I also have "fusion" methods like SequenceableCollection>> from: start to: finish allSatisfy: testBlock self from: start to: finish do: [:each | (aBlock value: each) ifFalse: [^false]]. ^true so that ((seq copyFrom: a to: z) allSatisfy: blk) can be done as (seq from: a to: z allSatisfy: blk) without making a copy. Fusion methods are useful because Smalltall compilers don't work as hard at eliminating intermediate data structures as functional language compilers. (Having other priorities.) (isdigit (last no_dashes return false if digit count > 10. On Tue, 8 Sep 2020 at 03:02, Steffen Märcker <merkste@web.de> wrote:

No problem. I am not knowledgeable about isbn numbers. At which places may a dash occur?

Kind regards, Steffen

07.09.2020 16:18:22 Roelof Wobben via Pharo-users < pharo-users@lists.pharo.org>:

...

Op 6-9-2020 om 10:07 schreef Steffen Märcker:

...

Maybe this is a naive question, but can you just split the task into the following two?

1. Check whether whether the string is syntactically an ISBN number. This can be done, e.g., using a regex.

2. Check the the check character. Calculate the check character from the (now to be known) syntactically valid string.

ISBNValidator>>isValidISBN: aString ^(self isSyntacticallyValid: aString) and: [self isCheckCharacterValid: aString]

Kind regards, Steffen nder if the code will not be too big. I learned that it is good

Sorry to respond not earlier but your respons seems to be in the spam folder of my provider.

I could do that but if very bad in regex so I do not know a regex which van validate 123456789 or 123-456-78-9

Roelof

Hi Richard and Roelof, thanks for your comprehensive answer. I brought up Regex only to point out alternative solutions. Another one is the following using transducers, where Tee works like the tee command from the command line. IsbnValidator>>isValidIsbn: aString | length countChars separators getSeparators lastChar getLastChar filterDigits computeCheckSum checkSum | "Count number of characters" length := 0. countChars := [:count :char | length := length + 1] init: length. "Get non-digit characters" separators := Set new getSeparators := separators <~ #isDigit remove. "Get last character" lastChar := nil. getLastChar := [:prev :char | lastChar := char ] init: lastChar. "Get digits" filterDigits := #isDigit filter. "Calculate check sum" computeCheckSum := ([:sum :index :digit | sum + index * digit value] init: 0) completing: #\\ . "Compute" checkSum := aString transduce: (Tee to: countChars) * (Tee to: getSeparators) * (Tee to: getLastChar) * filterDigits reduce: computeCheckSum init: 0. "Check validity" ^((length = 10 or: [length = 13]) and: [separators = Set with: $-]) and: [checkSum = (lastChar = $X ifTrue: [10] ifFalse: [lastChar value])] Kind regards, Steffen Am .09.2020, 08:30 Uhr, schrieb Roelof Wobben via Pharo-users <pharo-users@lists.pharo.org>:

Op 8-9-2020 om 04:22 schreef Richard O'Keefe:

...

There are two quite different questions. (1) Where may dashes occur in a real ISBN-10? (2) What does Exercism require in the specification and check in the test cases?

For (1) the rules are

Each ISBN consists of 5 elements with each section being separated by spaces or hyphens. Three of the five elements may be of >>varying length: Prefix element – currently this can only be either 978 or 979. It is always 3 digits in lengthRegistration group element – this identifies the particular country, geographical region, or language area participating >>in the ISBN system. This element may be between 1 and 5 digits in lengthRegistrant element - this identifies the particular publisher or imprint. This may be up to 7 digits in lengthPublication element – this identifies the particular edition and format of a specific title. This may be up to 6 digits >>in lengthCheck digit – this is always the final single digit that mathematically validates the rest of the number. It is >>calculated using a Modulus 10 system with alternate weights of 1 and 3.An ISBN-10 does not have the three-digit prefix. So we have [0-9]{1,5} -- prefix [0-9]{1,7} -- registrant [0-9]{1,6} -- publication [0-9X] -- check digit

As an examplw, "Standard C++ IOStreams and Locales" by Langer & Kreft has ISBN-10 0-201-18395-1 ISBN-13 9780201183955 so I shall assume the separators are optional. /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ Of course the elements cannot all have their maximum length at the same time. In AWK I would write x = a_putative_ISBN_10 y = x gsub(/[- ]+/, "", y) if (x ~ /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ \ && y ~ /^[0-9]{9,9}[0-9X]$/ \ ) { x *might* be valid, we still need to check the checksum }

For (2), there appear to be no restrictions on where dashes may occur or how many: "These may be communicated with or without hyphens". Exercism doesn't allow spaces.

Regular expressions are elegant in their own way, BUT for this problem they are (a) excessive, (b) inefficient, and (c) insufficient.

digit count := 0. check sum := 0. for each character c of the string if c is not a hyphen then if c is a digit then digit value := c's value as a digit else if c is X and digit count = 9 then digit value := 10 else return false. digit count := digit count + 1. if digit count > 10 then return false. check sum := (11 - digit count) * digit value + check sum. return check sum mod 11 is zero.

Part of the insight here is "don't DO it, just PRETEND you did." That is, instead of copying the string without the hyphens, just ignore the hyphens as they turn up. Another part is "if you are only going to use it once, don't store it." That is, we need a digit's value just once, in the update to check sum, so we should compute it just before we need it, not store it.

Now the pseudo-code above is classic sequential imperative coding.

Classic functional coding does something like let no_dashes = filter (/= '-') (explode string) in length no_dashes = 10 and let check = last no_dashes in (is_digit check or check = 'X') and all is_digit (take 9 no_dashes) and let xval c = if x = 'X' then 10 else digit_value c in dot (map xval no_dashes) [10,9..1]) mod 11 = 0

This pseudo-code translates nicely to Smalltalk too. You might want to add

SequenceableCollection>> with: other inject: initial into: aBlock |r| r := initial. self with: other do: [:x :y | r := aBlock value: r value: x value: y]. ^r dot: other ^self with: other inject: 0 into: [:acc :x :y | x*y + acc]

(These methods are so obvious that it would be absurd to claim any intellectual property rights to them.)

I also have "fusion" methods like SequenceableCollection>> from: start to: finish allSatisfy: testBlock self from: start to: finish do: [:each | (aBlock value: each) ifFalse: [^false]]. ^true

so that ((seq copyFrom: a to: z) allSatisfy: blk) can be done as (seq from: a to: z allSatisfy: blk) without making a copy.

Fusion methods are useful because Smalltall compilers don't work as hard at eliminating intermediate data structures as functional language compilers. (Having other priorities.)

(isdigit (last no_dashes return false if digit count > 10.

Thanks for letting me see this.But still I wonder if this is really the OOP way and if that function does more then 1 thing.It looks to me that its iterating trrough the string. Calculating the crc and checking it.I learned that it is a good thing that a function and a class does only 1 thing. Roelof

On Tue, Sep 8, 2020, 04:35 Roelof Wobben via Pharo-users < pharo-users@lists.pharo.org> wrote:

Op 8-9-2020 om 08:30 schreef Roelof Wobben:

Op 8-9-2020 om 04:22 schreef Richard O'Keefe:

There are two quite different questions. (1) Where may dashes occur in a real ISBN-10? (2) What does Exercism require in the specification and check in the test cases?

For (1) the rules are

Each ISBN consists of 5 elements with each section being separated by spaces or hyphens. Three of the five elements may be of varying length:

- *Prefix element* – currently this can only be either 978 or 979. It is always 3 digits in length - *Registration group element* – this identifies the particular country, geographical region, or language area participating in the ISBN system. This element may be between 1 and 5 digits in length - *Registrant element* - this identifies the particular publisher or imprint. This may be up to 7 digits in length

- *Publication element* – this identifies the particular edition and format of a specific title. This may be up to 6 digits in length - *Check digit* – this is always the final single digit that mathematically validates the rest of the number. It is calculated using a Modulus 10 system with alternate weights of 1 and 3.

An ISBN-10 does not have the three-digit prefix. So we have [0-9]{1,5} -- prefix [0-9]{1,7} -- registrant [0-9]{1,6} -- publication [0-9X] -- check digit

As an examplw, "Standard C++ IOStreams and Locales" by Langer & Kreft has ISBN-10 0-201-18395-1 ISBN-13 9780201183955 so I shall assume the separators are optional. /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ Of course the elements cannot all have their maximum length at the same time. In AWK I would write x = a_putative_ISBN_10 y = x gsub(/[- ]+/, "", y) if (x ~ /^[0-9]{1,5}[- ]?[0-9]{1,7}[- ]?[0-9]{1,6}[- ]?[0-9X]$/ \ && y ~ /^[0-9]{9,9}[0-9X]$/ \ ) { x *might* be valid, we still need to check the checksum }

For (2), there appear to be no restrictions on where dashes may occur or how many: "These may be communicated with or without hyphens". Exercism doesn't allow spaces.

Regular expressions are elegant in their own way, BUT for this problem they are (a) excessive, (b) inefficient, and (c) insufficient.

digit count := 0. check sum := 0. for each character c of the string if c is not a hyphen then if c is a digit then digit value := c's value as a digit else if c is X and digit count = 9 then digit value := 10 else return false. digit count := digit count + 1. if digit count > 10 then return false. check sum := (11 - digit count) * digit value + check sum. return check sum mod 11 is zero.

Part of the insight here is "don't DO it, just PRETEND you did." That is, instead of copying the string without the hyphens, just ignore the hyphens as they turn up. Another part is "if you are only going to use it once, don't store it." That is, we need a digit's value just once, in the update to check sum, so we should compute it just before we need it, not store it.

Now the pseudo-code above is classic sequential imperative coding.

Classic functional coding does something like let no_dashes = filter (/= '-') (explode string) in length no_dashes = 10 and let check = last no_dashes in (is_digit check or check = 'X') and all is_digit (take 9 no_dashes) and let xval c = if x = 'X' then 10 else digit_value c in dot (map xval no_dashes) [10,9..1]) mod 11 = 0

This pseudo-code translates nicely to Smalltalk too. You might want to add

SequenceableCollection>> with: other inject: initial into: aBlock |r| r := initial. self with: other do: [:x :y | r := aBlock value: r value: x value: y]. ^r dot: other ^self with: other inject: 0 into: [:acc :x :y | x*y + acc]

(These methods are so obvious that it would be absurd to claim any intellectual property rights to them.)

I also have "fusion" methods like SequenceableCollection>> from: start to: finish allSatisfy: testBlock self from: start to: finish do: [:each | (aBlock value: each) ifFalse: [^false]]. ^true

so that ((seq copyFrom: a to: z) allSatisfy: blk) can be done as (seq from: a to: z allSatisfy: blk) without making a copy.

Fusion methods are useful because Smalltall compilers don't work as hard at eliminating intermediate data structures as functional language compilers. (Having other priorities.)

(isdigit (last no_dashes return false if digit count > 10.

Thanks for letting me see this. But still I wonder if this is really the OOP way and if that function does more then 1 thing. It looks to me that its iterating trrough the string. Calculating the crc and checking it. I learned that it is a good thing that a function and a class does only 1 thing.

Roelof

I learned on other oop languages that for mainibility it's needed that a class does one thing and a method does also one thing. So when software changes , you can easily make the changes. Its I think called SOLID and I like that idea. So I try to make that also work in Pharo but it seems not so important. It's look like me that getting it work is more important.

I don't think that is the case here. Identify the objects involved in an ISBN string. I see String and Character. Once parsed, you could have an ISBN itself, with the elements cited by Richard O'keefe. But the exercise isn't about that, but only about the validation. There is very little behaviour that one could add to String and Character that would help in this exercise and that would be appropriate to their respective roles. Arguably, you could solve this exercise by creating a parser. I think that would be a lot like using a 10 pound sledge hammer to install a thumbtack.

Roelof