Proj2b Feedback

Here are some notes on the some of the clever, and not-so-clever, things you folks did for Project2b.

Unclever

Not handing in "XXX.st" was not clever. The assignment spec had a heading "what to hand in". Hint: you really should read a section with that sort of title.

Best. Comment Skipper. Ever

GraphReader.st

((words first) = commentChar)
    ifFalse: [self line: words for: graph]]]

Structuring the Draw

!Graph methods!
create 
    error = 1  "could have been a boolean"
        ifTrue: [ ^self error: 'Correct your error so the graph can print ' . ]
    self frame;
    	 label;    "<== example of cascade protocol"
    	 left;
    	 bottom;
    	 tag;
    	 data;
    	 mb;
    	 draw.

For she's a jolly good fellow

This code could have been a for-loop

 71     words second asInteger oo.
 72     (words second isInteger) isInteger
 73         ifFalse: [^self error: (line,')  expected Integer ')].
 74 
 75     words third asInteger oo.
 76     (words third isInteger) isInteger
 77         ifFalse: [^self error: (line,')  expected Integer ')].
 78 
 79     words fourth asInteger oo.
 80     (words fourth isInteger) isInteger 
 81         ifFalse: [^self error: (line,')  expected Integer ')].

For example:

words from: 2 to: 4 do: [:word|
	word isInteger isInteger 
		ifFalse: [^self error:( 'line s, ' at  word ' , i s . ') expected integer')]

A Class Act

Creating speciality classes. Hooray!

#DataPoint      isa: Object with: 'point symbol'.
#Line           isa: Object with: 'm b symbol'.

A Regular Guy

GraphReader.st

You used regular expressions!!!! well down

 28     (thing =~ '(^[0-9]+)') ifMatched: [
 29         words addFirst: thing.
 30         thing := 'data'].

Just do: it

The following code code be replaced with a for loop:

1 to: (arrayOfTag size) do: [:i | .... (arrayOfTag at: i) ..... ]

For example:

 
arrayOfTag do: [:tag | ... no need for arrayOfTag at: 1 ....]

Keep it Simple (#1)

Your init for datapoint could be a little simpler:

  2  new
  3   "auto-initialize new Ticks"
  4   | r |     
  5   r := super new.
  6   r init.       
  7   ^r

This could just be:

new
    ^super new init

Keep it Simple (#2)

 19 add: aNumber
 20   "in case the max is entered and then the min using add"
 21   |m|
 22   max = -100000 ifTrue: [
 23     min = 100000 ifFalse: [
 24         aNumber > min ifTrue: [
 25             m := min.
 26             max := m. 
 27             min := aNumber ]
 28                               ]
 29                         ].
 30   self min: aNumber;  max: aNumber.
 31 !   
 32 min
 33     ^min.
 34 !
 35 max
 36     ^max.
 37 !

Note that if you set init min and max to impossible large and small numbers (respectively) then life gets much simpler than in the above.

init
	min = 100000000.
	max = -1 * min.
!
add: aNumber
	self min: aNumber; max: aNumber
!
min: aNumber
	(aNumber < min) ifTrue: [min := aNumber]
!
max: aNumber
	(aNumber > max) ifTrue: [max := aNumber]
!