Hi Janet!

Janet Abdul-Karim <jn_karim at yahoo.com> wrote:
> I created a subclass to ordered collections.  I am trying to add objects to the new class and then print it to make sure they are in there.  I add one object it prints but when I add another object and print it only prints the first object i added
> 
> sample code to add to list.


First a few general remarks. Normally you don't need to make subclasses
of the Collection classes. They are instead meant to be used as
instances. An instance of a Collection class can generally be viewed as
a "bag"-object which you only use to hold multiple other objects.

Apart from this - let's have a look:

> account: aAccount
>  "Add an account to the portfolio"
> 
>  self isNil ifTrue:[self add: aAccount]
>   ifFalse:[ self addLast: aAccount].
> 

My first reaction is the name of the method. Simply by looking at the
name "account:" this looks like a so called "setter"-method. In
Smalltalk (and other OO languages) we talk about setter and
getter-methods. Such a method essentially only sets or gets an attribute
of the receiving object and the name is mostly synchronized with the
instance variable being set. So a typical "account:" method would look
like this:

account: anAccount
	myAccount := anAccount


...and that's it. It simply sets the instance variable "myAccount" to
refer to the same object that the parameter "anAccount" refers to.

So your method - which is actually adding an Account object to the
receiver object (which probably is a Portfolio object) should probably
be named "addAccount:" instead.

Finally you are probably trying to check if the receiver is empty and in
that case use "add:" instead of "addLast:". First of all this isn't
necessary - add: will work just fine - in fact if you look at the
implementation in OrderedCollection you will see that it simply always
calls "addLast:".

The check you are using "self isNil" is not the same as checking for
emptiness. Actually it will always evaluate to false because self is an
instance of OrderedCollection - not nil. :-)

In Smalltalk a variable (parameter, instance variable, self etc) always
points to an object. Yes, always. But what when it is "nil", you ask?
Well, in Smalltalk this situation is "solved" by simply having an object
that represents "nil"! So in Smalltalk we have one little lonely
instance of the class UndefinedObject and there is a "pseudo variable"
that always refers to him - "nil".

So when a variable is "uninitialized" in Smalltalk it is actually
pointing to the lonely nil guy - and you can actually send him messages,
though not very many.

> code to print list. 
> 
> ccounts
>  "Returns the accounts for the portfolio"
> 
>  self do:[:element|^element number].

And finally (which you probably would have wanted me to say first in
this email) we come to the "bug". This method is probably intended to
return a collection of Account numbers. The problem is that is has a
"return" statement in the block. This return will return from the method
when the do-block gets evaluated for the first element in self.

This is a working implementation:

^self collect: [:element | element number]


...the collect method is like "do:" but it collects the result of
evaluating the block for each element into a new collection. The return
statement is at the very beginning of the line and is the last thing
that will be executed thus returning the new collection instance that
the collect:-method returns.

Sorry if this posting got confusing. :-)

regards, Göran

PS. Let Portfolio inherit from Object instead of OrderedCollection and
give him an instance variable called "accounts". Let that instance
variable hold onto an instance of OrderedCollection and write the method
Portfolio>>addAccount: like:

addAccount: anAccount

	accounts isNil ifTrue: [accounts := OrderedCollection new].
	accounts add: anAccount

...as you can see I put in a so called "lazy initialization" here making
sure that the accounts variable is initialized to an empty collection if
it is "nil" before trying to add the account.