Smalltalk › Pharo › Pharo Smalltalk Users

Can it do this way ?

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

24 messages Options

Pharo Smalltalk Users mailing list

Sep 02, 2020; 6:18am

Can it do this way ?

914 posts

Hello,

I have now a challenge where I have to validate a ISBN number.

Can I do something like this on the class side :

(string isEmpty)
    ifTrrue: [ ^ false]
   ifFalse: [ digits:= something.
                    controlDigit := something.
                    self validateISBNNumber]

where on the validateISBNNumber I use the instance variables digits and
controlDigit to validate the ISBN number on the instance side.

Roelof

jtuchel

Sep 02, 2020; 6:41am

Re: Can it do this way ?

1922 posts

Roelof,

I don't think so.

A Class cannot access instance variables. Why? because a Class doesn't know which of its instances to ask for it.

Ask yourself: who is "self" in a Class method? Is it the Class or is it an individual instance of the Class?

(Hint: in a Class, #self is the Class, while in an instance, #self is the instance) .

"Who" do you want to validate the ISBN number? An individual instance of your Class (like Book)? Or is validating an ISBNNumber a Task that is independent of an individual item? If it is independent, then why would you want to use semething specific in that method?

You need to understand the differences between Class and Instance variables. I suggest reading Chapter 4 of "The Art and Science of Smalltalk" or better: read the whole book. It's easy to read and sparks quite a few A-HA's per hour.

HTH

Joachim

Am 02.09.20 um 08:18 schrieb Roelof Wobben:

Hello,

I have now a challenge where I have to validate a ISBN number.

Can I do something like this on the class side :

(string isEmpty)
   ifTrrue: [ ^ false]
ifFalse: [ digits:= something.
                   controlDigit := something.
                   self validateISBNNumber]

where on the validateISBNNumber I use the instance variables digits and controlDigit to validate the ISBN number on the instance side.

Roelof

... [show rest of quote]

-- 
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          [hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1

Richard O'Keefe

Sep 02, 2020; 9:25am

Re: Can it do this way ?

304 posts

Everything Joachim Tuchel wrote about this is fine EXCEPT that

this is an Exercism task, where Roelof has to implement an API

that he is given and is not at liberty to do it right.

Thanks to the way that IsbnVerifierTest is structured,

- the IsbnVerifier class has NO use for ANY variables of ANY

  kind: no instance variables, no class variables, and no

  class instance variables.

- the IsbnVerifier class has only ONE method that is called,

  and that is on the INSTANCE side.

That is, it's a simple pure function disguised as an object.

A good design might have an ISBN class, and would put

#isValidISBN10: and #isValidISBN13: on the class side.

But to solve the Exercism exercises, you HAVE to implement

the API that the test class is expecting, not something

better.  If it passes the tests, it is by definition right.

If it doesn't, it is by definition wrong, even if it is a

far better design.  So

 - one class, IsbnVerifier

 - with no variables of any kind

 - with one and only one method, #isValidIsbn,

   on the instance side.

It all gets so much clearer when you realise that the Exercism

tasks are NOT designed to teach Object-Oriented Design.  The

problems have to serve many programming languages, some of

which, like bash and x86 assembly code, are not object-oriented.

On Wed, 2 Sep 2020 at 18:42, [hidden email] <[hidden email]> wrote:

Roelof,

I don't think so.

A Class cannot access instance variables. Why? because a Class doesn't know which of its instances to ask for it.

Ask yourself: who is "self" in a Class method? Is it the Class or is it an individual instance of the Class?

(Hint: in a Class, #self is the Class, while in an instance, #self is the instance) .

"Who" do you want to validate the ISBN number? An individual instance of your Class (like Book)? Or is validating an ISBNNumber a Task that is independent of an individual item? If it is independent, then why would you want to use semething specific in that method?

You need to understand the differences between Class and Instance variables. I suggest reading Chapter 4 of "The Art and Science of Smalltalk" or better: read the whole book. It's easy to read and sparks quite a few A-HA's per hour.

HTH

Joachim

Am 02.09.20 um 08:18 schrieb Roelof Wobben:

Hello,

I have now a challenge where I have to validate a ISBN number.

Can I do something like this on the class side :

(string isEmpty)
   ifTrrue: [ ^ false]
ifFalse: [ digits:= something.
                   controlDigit := something.
                   self validateISBNNumber]

where on the validateISBNNumber I use the instance variables digits and controlDigit to validate the ISBN number on the instance side.

Roelof

... [show rest of quote]
-- 
-----------------------------------------------------------------------
Objektfabrik Joachim Tuchel          [hidden email]
Fliederweg 1                         http://www.objektfabrik.de
D-71640 Ludwigsburg                  http://joachimtuchel.wordpress.com
Telefon: +49 7141 56 10 86 0         Fax: +49 7141 56 10 86 1

... [show rest of quote]

Richard O'Keefe

Sep 02, 2020; 9:32am

Re: Can it do this way ?

304 posts

In reply to this post by Pharo Smalltalk Users mailing list

Briefly, you cannot and (for a different reason) you should not.

The IsbnVerifierTest class defines the API; the method must be

isValidIsbn: aString

and it MUST be on the instance side.

The reason why you shouldn't is that an empty string should NOT

be a special case.  This is a problem where the design style

described by Dijkstra in "A Discipline of Programming"

and further fleshed out by Reynolds and Gries in related books

pays off.  You want to inspect all the characters of the string

once each in a single left-to-right loop.

On Wed, 2 Sep 2020 at 18:19, Roelof Wobben via Pharo-users <[hidden email]> wrote:

Hello,

I have now a challenge where I have to validate a ISBN number.

Can I do something like this on the class side :

(string isEmpty)
    ifTrrue: [ ^ false]
   ifFalse: [ digits:= something.
                    controlDigit := something.
                    self validateISBNNumber]

where on the validateISBNNumber I use the instance variables digits and
controlDigit to validate the ISBN number on the instance side.

Roelof

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 02, 2020; 10:24am

Re: Can it do this way ?

914 posts

Yep, I know that isValidIsbn is the method that must output if a isbn is
valid or not.

What I want to do is take the first 9 characters out so I can convert
them to a array of numbers where I can do the calculation on.
And take out the last char so I can seperate test if that is a valid
char. So between the 0 and 9 or a X

I do not think I would have do all the checks in that only method
because it would be a very big method then.

but if I understand you well also the test if a string is empty should
be called from the isValidIsbn method or even checked there.

Roelof

Richard O'Keefe

Sep 02, 2020; 10:38am

Re: Can it do this way ?

304 posts

In reply to this post by 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.

On Wed, 2 Sep 2020 at 22:24, Roelof Wobben <[hidden email]> wrote:

Yep, I know that isValidIsbn is the method that must output if a isbn is
valid or not.

What I want to do is take the first 9 characters out so I can convert
them to a array of numbers where I can do the calculation on.
And take out the last char so I can seperate test if that is a valid
char. So between the 0 and 9 or a X

I do not think I would have do all the checks in that only method
because it would be a very big method then.

but if I understand you well also the test if a string is empty should
be called from the isValidIsbn method or even checked there.

Roelof

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 02, 2020; 10:47am

Re: Can it do this way ?

914 posts

In reply to this post by Richard O'Keefe

Correct. This is a exercism task.

and yes, there are no variables given but that does not in my oponion
means that I cannot add a instance or a class variable
I myself was thinking about adding two instance variables named digits
which hold the first 9 characters and a variable named controlDigit

Can I do it then this way

1) Check if the string is empty , if so return false otherwise made a
new object like this in IsValidIsbn

isValidIsbn: aString
     if aString isEmpty
       ifTrue: [False]
      ifFalse: [ self new
                          digits := something.
                          controlDigit := something.]

I can also do what I think you are describing

isValidIsbn: AString
   if aString isEmpty
        ifTrue: [ ^false]
        ifFalse: [ISBN isValidIsbn10]

and then as you describe so have a isValidISBN10 method in the class
side where I do the check there so the class ISBN has the two instance
variables digits and controlDigit

Roelof

Pharo Smalltalk Users mailing list

Sep 02, 2020; 10:52am

Re: Can it do this way ?

914 posts

In reply to this post by Richard O'Keefe

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.

... [show rest of quote]

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

Pharo Smalltalk Users mailing list

Sep 03, 2020; 4:12pm

Re: Can it do this way ?

914 posts

Op 2-9-2020 om 12:52 schreef Roelof Wobben via Pharo-users:

So no more hints from other people?

Roelof

Richard O'Keefe

Sep 04, 2020; 3:24am

Re: Can it do this way ?

304 posts

In reply to this post by 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 <[hidden email]> 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.

... [show rest of quote]

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

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 04, 2020; 4:45am

Re: Can it do this way ?

914 posts

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 <[hidden email]> 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.

... [show rest of quote]

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

... [show rest of quote]

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 04, 2020; 5:35am

Re: Can it do this way ?

914 posts

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 <[hidden email]> 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.

... [show rest of quote]

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

... [show rest of quote]

... [show rest of quote]

... [show rest of quote]

Steffen Märcker

Sep 06, 2020; 8:07am

Re: Can it do this way ?

381 posts

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
<[hidden email]>:

> 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 <[hidden email]> 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
>
>
>

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 07, 2020; 5:08am

Re: Can it do this way ?

914 posts

In reply to this post by Pharo Smalltalk Users mailing list

Hello,

I solved it but with what I find one big ugly code

isValidIsbn: aString
    | digits lastDigit acc |
    digits := aString select: [ :each | each ~= $- ].
    digits size = 10
        ifFalse: [ ^ false ].
    lastDigit := digits last.
    digits := aString allButLast asOrderedCollection
        select: [ :each | each isDecimalDigit ]
        thenCollect: [ :each | each digitValue ].
    ((lastDigit = $X or: [ lastDigit isDigit ]) and: [ digits size == 9 ])
        ifFalse: [ ^ false ].
    acc := digits
        with: (10 to: 2 by: -1)
        collect: [ :digit :multiplier | digit * multiplier ].
    ^ ((acc sum + (lastDigit digitValue min: 10)) % 11) isZero

Can this be improved some way ?

Roelof

Op 4-9-2020 om 07:35 schreef Roelof Wobben via Pharo-users:

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 <[hidden email]> 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.

... [show rest of quote]

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

... [show rest of quote]

... [show rest of quote]

... [show rest of quote]

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 07, 2020; 2:17pm

Re: Can it do this way ?

914 posts

In reply to this post by Steffen Märcker

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
>

... [show rest of quote]

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

Steffen Märcker

Sep 07, 2020; 3:01pm

Re: Can it do this way ?

381 posts

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 <[hidden email]>:

> 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
>

... [show rest of quote]

Pharo Smalltalk Users mailing list

Sep 07, 2020; 4:19pm

Re: Can it do this way ?

914 posts

See here for all the tests :
https://github.com/exercism/pharo-smalltalk/blob/master/exercises/isbn-verifier/IsbnVerifierTest.class.st#L88

Roelof

Steffen Märcker

Sep 07, 2020; 5:01pm

Re: Can it do this way ?

381 posts

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
<[hidden email]>:

> See here for all the tests :
> https://github.com/exercism/pharo-smalltalk/blob/master/exercises/isbn-verifier/IsbnVerifierTest.class.st#L88
>
> Roelof
>
>
>

Steffen Märcker

Sep 07, 2020; 8:14pm

Re: Can it do this way ?

381 posts

Now having a Workspace at hand, I fixed some minor typos:

IsbnVarifier>>isSyntacticIsbn: aString
| nonGrouped dashes spaces grouped |
nonGrouped := '\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 := (dashes , '|' , spaces) asRegex.
^(aString matchesRegex: nonGrouped) or:
[(aString matchesRegex: grouped) and:
[aString size = (10 + 3)]]

Best, Steffen

Am .09.2020, 19:01 Uhr, schrieb Steffen Märcker <[hidden email]>:

> 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
> <[hidden email]>:
>
>> See here for all the tests :
>> https://github.com/exercism/pharo-smalltalk/blob/master/exercises/isbn-verifier/IsbnVerifierTest.class.st#L88
>>
>> Roelof
>>
>>
>>

... [show rest of quote]

Richard O'Keefe

Sep 08, 2020; 2:22am

Re: Can it do this way ?

304 posts

In reply to this post by Steffen Märcker

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 <[hidden email]> 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 <[hidden email]>:

> 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
>

... [show rest of quote]