CODEWITHSUNDEEP
smalltalk
FlapsFail
FlapsFail

Reputation: 525

Smalltalk syntax error

I'm learning smalltalk right now and am trying to make a very simple program that creates an array of numbers and then finds the largest number. My code looks like this:

| list max |
list := #(1 8 4 5 3).
    1 to: list size do: [:i |
    max < (list at: i)
        ifTrue: [max := (list at: i)].
        ifFalse: [max := max].
    ].

When I run this code, I get "stdin:7: parse error, expected ']'". I'm a bit confused as to what is causing this. It looks to me like all of my square brackets correspond. Help?

Upvotes: 1

Views: 2008

Answers (3)

Norbert Hartl
Norbert Hartl

Reputation: 10841

Alexandre told you already that is likely that collection provides a max method. You might be interested in a few ways how to do it.

Using collection max (maximum of all elements)

#(1 8 4 5 3) max

Using number max: (which of two numbers is bigger)

#(1 8 4 5 3) inject: 0 into: [:max :elem|
     max max: elem ]

Or using just using the internal iterator

#(1 8 4 5 3) inject: 0 into: [:max :elem|
    max < elem 
       ifTrue: [ elem ]
       ifFalse:[ max ] ]

Together with your solution to use an external iteration you can see there are a lot of possibilities.

Hope it adds something

Upvotes: 5

paxdiablo
paxdiablo

Reputation: 881403

Your immediate problem is that you terminate the ifTrue section with a period. You could normally just remove the period but, since your ifFalse section is effectively a non-operation, it's probably better to remove that bit.

But even when you fix that, you still need to initialise max so that you don't get a < message being sent to the nil object. You can initialise it to the first element, if there are any, then you can change the loop to start at the second.

Of course, initialising to the first element is problematic when the list is empty so you should handle that as well. In the code below, I've set it to a suitable small value then initialised it from the first element of the list only if it's available.

Corrected code is:

| list max |
list := #(1 8 4 5 3).
max := -99999.
(list size) > 0 ifTrue: [max := (list at: 1)].
2 to: list size do: [:i |
    max < (list at: i) ifTrue: [max := (list at: i)].
]
(max) displayNl

This outputs 8 as expected and also works fine on the edge cases (list size of zero and one).

Upvotes: 3

Alex Jasmin
Alex Jasmin

Reputation: 39496

The line number seems off but looking at your code it seems the error is probably caused by the period after ifTrue: [max := (list at: i)]. #ifTrue:ifFalse is a single method selector and it doesn't make sense to break it into two statements.

Actually you could remove the ifFalse part of the code entirely. Assigning max to itself has no effect. Also looping on the indices is not needed here. You can work on the values directly with list do: […].

The max variable should also be initialized. Zero seems like a good choice to compare against the positive numbers in your array.

But instead of doing all that look into the Collection class. Your Smalltalk dialect may already offer a max method for this task.

Upvotes: 4

Related Questions