smalltalk

Overrides - useful, but dangerous

February 9, 2005 12:39:27.949

One of the interesting capabilities of VisualWorks is overriding. Now, this doesn't mean simply redoing a method in a subclass - no, it means something that is potentially more powerful - but also more dangerous. What you can do is redefine a method in an existing class in a loadable component - either a Store package or a Parcel. You have been able to do this since the beginning with Smalltalk - all overrides do is add some traceability to the process. let me give a small example:

For BottomFeeder, we use a network component called NetResources, a library built on top of the VisualWorks HTTP client support. We needed a higher level library to support things like mod-gzip that the client lib did not (yet) support. In any case, as part of the implementation, we overrode the definition of the class HttpClient. Here's the VW 7.2.1 definition:


Smalltalk.Net defineClass: #HttpClient
	superclass: #{Net.NetClient}
	indexedType: #none
	private: false
	instanceVariableNames: 'request proxyHost keepAlive connectionStream useProxyAuthorization '
	classInstanceVariableNames: ''
	imports: ''
	category: 'Net-HTTP-Support'

In contrast, here's the overridden definition we used in VW 7.2.1:


Smalltalk.Net defineClass: #HttpClient
	superclass: #{Net.NetClient}
	indexedType: #none
	private: false
	instanceVariableNames: 'request proxyHost keepAlive connectionStream useProxyAuthorization originalResponse '
	classInstanceVariableNames: ''
	imports: ''
	category: 'Net-HTTP-Support'

We added an instance variable in order to have better error handling. This was fine, and it all worked without any issues before we started the port to VW 7.3 (which went live in December - see here for details). Here's the 7.3 definition of that class:


Smalltalk.Net defineClass: #HttpClient
	superclass: #{Net.NetClient}
	indexedType: #none
	private: false
	instanceVariableNames: 'request proxyHost keepAlive useProxyAuthorization connection entityParsingOptions cookieAgent enableCookies protocol '
	classInstanceVariableNames: ''
	imports: ''
	category: 'Net-HTTP-Support'

As you can see, a number of new instance variables were defined (and the class now natively handles mod-gzip). We took out our mod-gzip handling, but we didn't immediately notice the more dangerous thing - our (obsolete) class definition did not have any of the new instance variables - and they are used in the code. Well, that led to some bizarre errors (in development). As it happens, VW will deal with undeclared references by creating a variable in the Undeclared dictionary. After running Bf in development a few times, I had a lot of state in Undeclared. The trouble is, when running threaded Http accesses, multiple clients ended up sharing things that they had no business sharing! Fixing up the definition solved that problem

The bottom line? Overrides are useful, but - with each migration to a new release you have to carefully examine your overrides to see if they are still congruent with the system. In our case, they weren't. It's useful to be able to patch problems in place, and it's also easy to trace - I always store system overrides into a separate package so that I know where to look. The key is - you have to actually look

Comments

Overrides - useful, but dnagerous

[Tom Sattler] February 10, 2005 13:27:08.206

Interesting problem, and one I have had bite me on several occassions. I submit that a reference to an instance variable that does not exist should NEVER EVER WORK. It should not create an entry in Undeclared. We don't use Globals in Smalltalk (yes, I know they exist, but they shouldn't). It's like #instVarAt: - yes, you can do it, but don't. Just don't.

All this does is mask a development problem. The developer screwed up, and the system should not save him from the consequences. I submit that the system should have failed, with a message telling the developer what was wrong. IF you don't want it to fail, put a message in a log somewhere, perhaps "LookHereDummy.txt" or some such. But just allowing the image to hum merrily along as if everything was peachy is not the best way to handle this.

 Share Tweet This
-->