Smalltalk Input/Output

Question

I am having trouble regarding Smalltalk. I am attempting to populate an array with the numbers that are read from the file, but it doesn't seem to work. I've tried numerous options and I was hoping someone would explain to me what I'm doing wrong.

 Object subclass: #MyStack
        instanceVariableNames:'anArray aStack'
        classVariableNames:''
        poolDictionaries:''

!
MyStack class comment: 'Creates a Stack Class.'
!

!
MyStack methodsFor: 'initialize Stack'
!

new "instance creation"
        ^ super new.
!
init "initialization"
        anArray := Array new: 32.
        aStack := 0.
! !

!MyStack methodsFor: 'methods for stacks' !


pop "Removes the top entry from the stack"
    | item |
    item := anArray at: aStack.
    aStack := aStack - 1.
!

push: x "Pushes a new entry onto the stack"
    aStack := aStack + 1.
    anArray at:aStack put:x.
!

top "Returns the current top of the stack"
        ^anArray at: aStack.
!

empty "True if the stack is empty"
        ^aStack = 0.
!

full "True if the stack is full"
        ^aStack = 32.
!

printOn: aStream "Prints entire stack one entry per line, starting the top entry"
    aStream show: 'Stack:'.
    aStack to:1 by:-1 do:[:i |(anArray at:i) printOn:aStream. ].
    aStream show: ''
! !
"----------------------------------------------------------------------------------"
Object subclass: #IOExample
  instanceVariableNames: 'input output'
  classVariableNames: ''
  poolDictionaries: ''
!
IOExample class comment: '
 basic I/O.
'
!

!
IOExample methodsFor: 'initialize'
!
new
  ^ super new.

!
init
  [ input := FileSelectionBrowser open asFilename readStream. ]
    on: Error
    do: [ :exception |
      Dialog warn: 'Unable to open file'.
      exception retry.
    ].
  [ output := FileSelectionBrowser open asFilename writeStream. ]
    on: Error
    do: [ :exception |
      Dialog warn: 'Unable to open file'.
      exception retry.
    ].

! !

!
IOExample methodsFor: 'copy input to output turning :: into :'
!
copy
  | data lookAhead theStack myStack|
  [ input atEnd ] whileFalse: [

    data := input next.
    (data isKindOf: Integer)
      ifTrue: [
        (input atEnd) ifFalse: [
        "myStack push: data."
        lookAhead = input peek.
        (lookAhead asCharacter isDigit)
            ifTrue: [
            ]              
        ].
      ].  
    output show: myStack.



  ].
  input close.
  output close.
! !

Answer 1

Justin, let me try to help you with the class MyStack and defer to another answer any comments on your example.

I've divided your code into fragments and appended my comments.

---

Fragment A:

Object subclass: #MyStack
    instanceVariableNames:'anArray aStack'
    classVariableNames:''
    poolDictionaries:''

Comments for A:

A Smalltalker would have used instance variable names without indeterminate articles a or an

Object subclass: #MyStack
    instanceVariableNames:'array stack'
    classVariableNames:''
    poolDictionaries:''

---

Fragment B:

MyStack class comment: 'Creates a Stack Class.'

Comments for B:

This is weird. I would have expected this instead (with no class):

MyStack comment: 'Creates a Stack Class.'

---

Fragment C:

MyStack methodsFor: 'initialize Stack'

new "instance creation"
    ^ super new.

Comments for C:*

This code puts new on the instance side of the class, which makes no sense because you usually send new to the class rather than its instances. The correct form requires adding class:

MyStack class methodsFor: 'initialize Stack'

new
    ^super new.

You forgot to send the initialization method (however, see Fragment D below)

new
    ^super new init.

---

Fragment D:

init "initialization"
    anArray := Array new: 32.
    aStack := 0.

Comments for D:

In Smalltalk people use the selector initialize so it can send super first

initialize
    super initialize.
    array := Array new: 32.
    stack := 0.

Note that this change would require also writing new as

new
    ^super new initialize.

However, if your dialect already sends the initialize method by default, you should remove the implementation of new from your class.

---

Fragment E:

pop "Removes the top entry from the stack"
    | item |
    item := anArray at: aStack.
    aStack := aStack - 1.

Comments for E:

You forgot to answer the item just popped out

pop
    | item |
    item := array at: stack.
    stack := stack - 1.
    ^item

---

Fragment F:

push: x "Pushes a new entry onto the stack"
    aStack := aStack + 1.
    anArray at:aStack put:x.

Comments for F:

This is ok. Note however that the stack will refuse to push any item beyond the limit of 32.

push: x
    stack := stack + 1.
    array at: stack put: x.

---

Fragment G:

top "Returns the current top of the stack"
    ^anArray at: aStack.

empty "True if the stack is empty"
    ^aStack = 0.

full "True if the stack is full"
    ^aStack = 32.

Comments for G:

These are ok too. However, a more appropraite name for empty would have been isEmpty because all collections understand this polymorphic message. Similarly, the recommended selector for full would be isFull:

top
    ^array at: aStack.

isEmpty
    ^stack = 0.

isFull
    ^stack = 32.

Note also that isFull repeats the magic constant 32, which you used in the initialization code. That's not a good idea because if you change your mind in the future and decide to change 32 with, say, 64 you will have to modify two methods an not just one. You can eliminate this duplication in this way

isFull
    ^stack = array size.

---

Fragment H:

printOn: aStream
    "Prints entire stack one entry per line, starting the top entry"
    aStream show: 'Stack:'.
    aStack to:1 by:-1 do:[:i |(anArray at:i) printOn:aStream. ].
    aStream show: ''

Comments for H:

The last line of this code is superfluous and I would get rid of it. However, you may want to separate every item from the next with a space

printOn: aStream
    stream show: 'Stack:'.
    stack to: 1 by: -1 do:[:i |
        aStream space.
        (array at: i) printOn: aStream].

Answer 2

Did you try to run this code? If you did, I'm surprised you didn't get a compilation warning due to #2 below.

There are a number of problems in #copy (besides the fact that I don't understand exactly what it's trying to do)...

1. First you seems to expect the data to be numbers: data isKindOf: Integer. But then later you treat it as a stream of Characters: lookAhead asCharacter isDigit. If the first condition is true to get you past that point, the second one never can be, as you would've matched [0-9], which aren't ASCII values for digits.
2. lookAhead = input peek. Here you're comparing uninitialized lookAhead (nil) with the peeked value, and then throwing away the result. I assume you meant lookAhead := input peek.
3. Then there is the empty inner condition ifTrue: [ ]. What are you trying to do there?
4. Then there's the odd protocol name, 'copy input to output turning :: into :'. What does that mean, and what does that have to do with copying numbers between streams?