Small problem in porting 4.0 package to 5.1

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Small problem in porting 4.0 package to 5.1

Ole M Halck
Recently, I loaded one of my D4.0 applications in package form into
D5.1. Loading the package itself went without problems, but when I
tried to start the thing (evaluating "MyShell show"), I got the error
message "key cannot be nil". It turned out that D5 introduces a line

key isNil ifTrue: [^self error: 'key cannot be nil']

in SharedLookupTable>>#at:put:, to ensure that the "at:" argument is
not nil.

The error appeared to occur when setting up event messages to be sent
between view/presenter and model, and is probably due to the fact that
some subview/-presenters start life without a model connected.

Removing the line in SharedLookupTable and also in LookupTable made
everything run smoothly. But is there a danger attached to doing this?
After all, I suspect OA had a reason to introduce these checks in the
first place?

Thanks to anyone who can shed some light on this for me.
--
OleM


Reply | Threaded
Open this post in threaded view
|

Re: Small problem in porting 4.0 package to 5.1

Blair McGlashan
Ole

You wrote in message news:[hidden email]...

> Recently, I loaded one of my D4.0 applications in package form into
> D5.1. Loading the package itself went without problems, but when I
> tried to start the thing (evaluating "MyShell show"), I got the error
> message "key cannot be nil". It turned out that D5 introduces a line
>
> key isNil ifTrue: [^self error: 'key cannot be nil']
>
> in SharedLookupTable>>#at:put:, to ensure that the "at:" argument is
> not nil.
>
> The error appeared to occur when setting up event messages to be sent
> between view/presenter and model, and is probably due to the fact that
> some subview/-presenters start life without a model connected.
>
> Removing the line in SharedLookupTable and also in LookupTable made
> everything run smoothly. But is there a danger attached to doing this?
> After all, I suspect OA had a reason to introduce these checks in the
> first place?
>...

Yes, there is a danger. If you remove the guard clause, and then add
something to a LookupTable/Dictionary with a nil key, then that value will
effectively be orphaned. You can't lookup a nil key in a LookupTable because
nil is used to mark empty slots. In relation to events, this means that the
event registration is effectively lost. This would suggest there is
something awry with your application that was previously not noticed, even
if the bug is benign. FWIW this is the first time we have heard of this
check firing (at least no-one else has reported it, and we didn't come
across it ourselves with our own packages)

Regards

Blair


Reply | Threaded
Open this post in threaded view
|

Re: Small problem in porting 4.0 package to 5.1

Bill Schwab-2
Blair,

> Yes, there is a danger. If you remove the guard clause, and then add
> something to a LookupTable/Dictionary with a nil key, then that value will
> effectively be orphaned. You can't lookup a nil key in a LookupTable
because
> nil is used to mark empty slots. In relation to events, this means that
the
> event registration is effectively lost. This would suggest there is
> something awry with your application that was previously not noticed, even
> if the bug is benign. FWIW this is the first time we have heard of this
> check firing (at least no-one else has reported it, and we didn't come
> across it ourselves with our own packages)

I have not hit it in the event system, but I have had problems with other
uses of dictionaries.  IMHO, it would be better to create a cookie for empty
slots, and allow nil to be used as a key.  Caveat: tell me there is big
performance or other benefit to nil as the cookie, and I'll gladly eat those
words :)

Have a good one,

Bill

--
Wilhelm K. Schwab, Ph.D.
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Small problem in porting 4.0 package to 5.1

Blair McGlashan
"Bill Schwab" <[hidden email]> wrote in message
news:[hidden email]...
> Blair,
>
> > Yes, there is a danger. If you remove the guard clause, and then add
> > something to a LookupTable/Dictionary with a nil key, then that value
will
> > effectively be orphaned. You can't lookup a nil key in a LookupTable
> because
> > nil is used to mark empty slots. In relation to events, this means that
> the
> > event registration is effectively lost. This would suggest there is
> > something awry with your application that was previously not noticed,
even
> > if the bug is benign. FWIW this is the first time we have heard of this
> > check firing (at least no-one else has reported it, and we didn't come
> > across it ourselves with our own packages)
>
> I have not hit it in the event system, but I have had problems with other
> uses of dictionaries.

'nil' has never worked as a key in dictionaries (or an element in Sets for
that matter), either in Dolphin or any other Smalltalk that I am familar
with*. The error check in D5.1 (which is the subject of the original
posting) simply prevents the error from going unnoticed.

*Actually that is not quite true. It does work in instances of Dictionary
(at least if one removes the test that raises an error for nil keys),
because these store Associations in the slots, and hence one can distinguish
between full and empty slots irrespective of the key since only truly empty
slots would contain nil. However it will not work for any kind of
LookupTable (or IdentityDictionary), since these store the keys and values
in separate vectors, and so one cannot distinguish a nil key from any empty
slot. What this means is that in some Smalltalk's (VisualSmalltalk being
one) it was possible to use nil as a key in a Dictionary, but it was an
error in an IdentityDictionary. This inconsitency in behaviour seemed
unhelpful to us, so we opted to disallow nil keys in any dictionary, even
though the implementation of Dictionary itself would allow for nil keys.

>...IMHO, it would be better to create a cookie for empty
> slots, and allow nil to be used as a key.

The long established Smalltalk-80 behaviour seems like a reasonable
pragmatic implementation decision to me. If one considers what nil is
supposed to represent (i.e. the undefined object), then it makes sense to
use it to represent empty slots and disallow it as a key. The ANSI standard
simply says that the results of using 'nil' as a key in a dictionary are
"undefined". This might mean that there must have been one or more Smalltalk
system existing at the new time which did allow nil keys, or more likely
that there was one or more which simply didn't check for the invalid use of
nil keys. Regardless, what it tells you is that you cannot rely on being
able to use nil as a key in standard Smalltalk.

>...Caveat: tell me there is big
> performance or other benefit to nil as the cookie, and I'll gladly eat
those
> words :)

There would certainly be a performance hit, since many operations against
nil (testing it, setting it, etc) are faster than comparison against an
arbitrary object. Would these costs be significant? Well I haven't attempted
to measure it, but for the most part it would probably be small in absolute
terms per operation (the overhead of allocating or growing a dictionary
which used some other cookie to mark empty slots would be more significant,
since the object memory initializes to nil). Of course the dictionaries are
so widely used that any performance gain or loss would be fealt in almost
every library and application.

Regards

Blair