On Sun, Apr 26, 2020 at 10:11 AM Roelof Wobben via Pharo-users <
pharo-users@lists.pharo.org> wrote:
> Hello,
>
> I have to make some code that convert a binary to a decimal and im not
> allowed to use the convert methods that Pharo has.
>
> So I have written this :
>
>
> decimalFromBinary: aString
> | result isValid |
> isValid = self isValidBinary: aString.
> isValid
> ifTrue: [ result := 0.
> aString reverse
> withIndexDo:
> [ :digit :index | result := result + (digit
> digitValue * (2 raisedTo: index - 1)) ].
> ^ result ]
> ifFalse: [ ^ nil ]
>
> isValidBinary: aString
> ^ aString allSatisfy: [ :c | c = 0 or: [ c = 1 ] ]
>
Others have answered your question, so I'll limit my response to other
aspects of this exercise.
Answering nil is a bad practice, in general. There will be occasions where
it is correct, if non-existence really is part of the model. Answering nil
may have been a requirement of the exercise and if that's the case, you
have no choice. By answering nil, you force every sender to check for nil
rather than being able to rely on receiving an Integer result. Even with
method selectors that tell the reader a nil can be the result, too many
people will fail to check the result before using it. In general, I prefer
to have the validation throw an exception.
e.g.
validateBinaryString: aString
(aString allSatisfy: [:each | '01' includes: each])
ifFalse: [self error: aString printString, ' is not a valid
representation of a binary number'].
The method simply returns self if it succeeds or throws an exception is the
string is invalid. And likewise, the conversion method itself only answers
the integer corresponding to a valid binary string and does not answer
(anything) for an invlaid one.
There is another pattern that could be used, but this particular problem
seems (to me) best suited to an exception. A common pattern in Smalltalk,
one of its greatest innovations, is the #ifAbsent: keyword and others like
it. When you send #at:ifAbsent:, *you* tell the receiver to give you what
you ask for *and* you tell it what *you* want to do if it cannot. There are
infinitely many possible keywords like #ifAbsent:, as you can make up any
one that suits your needs.
If you look at the implementation of #at: or #detect:, you will see that
they are implemented using #at:ifAbsent: and #detect:ifNone: and that the
block provided for the second argument is one which throws an error. Given
all that, perhaps the "best" implementation of your solution would be to
have "decimalFromBinary: aString" implemented in terms of a
"decimalFromBinary: aString ifUnable: aBlock".
>
> but on the first method I see a message that the temp variables are read
> before written.
> and the second one I see a message that I use a or instead of searching
> literals.
>
> Where did I think wrong here ?
>
> Roelof
>
>
>
