Hi Goran,
Thanks for the walk through of a refactoring. I'll put this on the Swiki.
With further testing I found two bugs with my original code:
1) 2 asFraction printStringAsMixedNumber. prints as 20
(Jeff Bowman also caught that.)
2) (3/1) printStringAsMixedNumber causes a walkback window because
(3/1) reduces to the SmallInteger 3. 

The attached changeset fixes both problems. I'll incorporate your ideas
after I have a chance to study them.

Mike

On Wednesday 06 March 2002 01:40 am, goran.hultgren at bluefish.se wrote:
> Michael Maloney <amaloney at earthlink.net> wrote:
> > Well, all the books have said to start small, so . . .
>
> Exactly! After all - it is SMALLtalk. :-)
>
> > In case anyone else finds it useful, here is a method I've added
> > to Fraction.
> >
> > "(5/3) printStringAsMixedNumber" prints as 1(2/3).
> >
> > The newbie part is: Have I followed accepted practice
> > as far as naming convention and putting the method in
> > the printing protocol?
>
> Think so. I might have named it #printAsMixedNumber, but that is mere
> taste.
>
> > Also, (especially) since I modified a kernal class, should I
> > have named it something else?
>
> Nope, seems fine to me. A few advice:
>
> 1. Learn to use changesets instead of .st files. They are a bit more
> useful.
>
> 2. When adding "printYaddaYadda" methods I usually tend to prefer the
> form that takes a stream as an argument (as #printOn:) - it can make the
> method a bit more useful. More on that below.
>
> 3. Your line that goes:
> 	text _ '' , wholePart printString , fractionPart printString.
> ...can instead be written as:
> 	text _ wholePart printString , fractionPart printString.
> There is no need to concatenate with an empty String at the beginning. I
> am not sure why you did that.
>
> Ok, follow me on a little "excursion" here - this is not meant as
> criticism - hopefully there is something to be learned from it. Let's
> change the method a teeny bit first:
>
> printStringAsMixedNumber
>
> 	| wholePart fractionPart |
>
> 	wholePart _ self truncated.
> 	fractionPart _ self - wholePart.
> 	^wholePart printString , fractionPart printString
>
> The return is always "evaluated" last so you can always safely put it at
> the beginning of the line and you don't have to use parentheses. You
> also don't need to end the final statement in the method with the
> .-character. It is a "statement separator" not a "statement endmarker".
> On the other hand it can be there without errors.
>
> Let's go a bit further:
> printStringAsMixedNumber
>
> 	| wholePart |
>
> 	wholePart _ self truncated.
> 	^wholePart printString , (self - wholePart) printString
>
> When examining the implementation of #printString (select it and press
> Alt-m) we see that it is actually a "limited" version. In this case
> perhaps #fullPrintString would be better - at least we get rid of two
> extra message sends! :-) (Yeah, yeah, permature optimization is the root
> of all evil which Donald Knuth didn't say btw).
>
> So currently we have:
> printStringAsMixedNumber
>
> 	| wholePart |
>
> 	wholePart _ self truncated.
> 	^wholePart fullPrintString , (self - wholePart) fullPrintString
>
> ...if we then take a look at fullPrintString (Alt-m) we find it
> implemented like this:
> ^String streamContents: [:s | self printOn: s]
>
> The method streamContents: is kind of new in Smalltalk I think. I just
> checked the versions of it (Alt-v) but I found no reference to when it
> was introduced. It is a nice method though and makes it easy to produce
> a String by using methods that wants to write on a stream.
>
> Hmmm. So then we might end up with my suggestion for a stream-based
> implementation like this:
> printAsMixedNumberOn: aStream
>
> 	| wholePart |
>
> 	wholePart _ self truncated.
> 	wholePart printOn: aStream.
> 	(self - wholePart) printOn: aStream
>
> This method takes a Stream as an argument and writes the representation
> on the stream. The nice part with this is that the Stream can be any
> kind of Stream like for example a FileStream.
>
> ...and if you still want a method returning a String you can always add
> an extra method like this:
> printStringAsMixedNumber
> 	^String streamContents: [:stream | self printAsMixedNumberOn: stream]
>
>
> And here ends my little excursion. :-)
>
> > Cheers,
> > Mike
>
> Cheers, Göran

-------------- next part --------------
'From Squeak3.2gamma of 3 March 2002 [latest update: #4743] on 6 March 2002 at 11:31:16 pm'!

!Fraction methodsFor: 'printing' stamp: 'amm 3/6/2002 22:35'!
printStringAsMixedNumber

	| text wholePart fractionPart |
	wholePart _ self truncated.
	fractionPart _ self - wholePart.
	(fractionPart > 0) 
		ifTrue: [text _ wholePart printString,fractionPart printString]
	 	ifFalse: [text _ wholePart printString].
	^ text.	! !


!Integer methodsFor: 'printing' stamp: 'amm 3/6/2002 22:53'!
printStringAsMixedNumber
	"Handle the case where a fraction reduces to a whole number"
	self printString
! !