Why is the Pharo definition of arithmetic + and - basically duplicated code?

Question

I was looking through the internals of Pharo and noticed that the definition of arithmetic + and - look very much alike:

+ aNumber
"Refer to the comment in Number + "
aNumber isInteger ifTrue:
    [self negative == aNumber negative
        ifTrue: [^ (self digitAdd: aNumber) normalize]
        ifFalse: [^ self digitSubtract: aNumber]].
aNumber isFraction ifTrue:
    [^Fraction numerator: self * aNumber denominator + aNumber numerator denominator: aNumber denominator].
^ aNumber adaptToInteger: self andSend: #+

and

- aNumber
"Refer to the comment in Number - "
aNumber isInteger ifTrue:
    [self negative == aNumber negative
        ifTrue: [^ self digitSubtract: aNumber]
        ifFalse: [^ (self digitAdd: aNumber) normalize]].
aNumber isFraction ifTrue:
    [^Fraction numerator: self * aNumber denominator - aNumber numerator denominator: aNumber denominator].
^ aNumber adaptToInteger: self andSend: #-

As I see it, this is completely against the OO way of designing things and is generally bad. Why doesn't anybody find a better solution?

Answer 1

The simplest thing I can think of is:

- aNumber
    ^self + aNumber negated

However, this will have a cost:

• creation of another intermediate LargeInteger, or Fraction
• two more message sends to perform the - operation

What we see here is a tribute to optimization. Not premature optimization, this is a low level operation used extensively.

There are other things in this code which are not perfect:

• usage of isInteger and isFraction might as well be replaced with some kind of double-dispatching
• the methods digitAdd: and digitSubtract: work for integers stored as sign - magnitude rather than 2-complement which is a not completely obvious implementation detail and would deserve a comment - or maybe should better be renamed digitAddMagnitude: digitSubtractMagnitude: