Please use accessors!

Alan Lovejoy sourcery at pacbell.net
Sun Aug 1 00:40:49 UTC 1999


> ** Original Sender: "Carl Gundel" <morphic at hotmail.com>
> >From: Alan Lovejoy <sourcery at pacbell.net>
> >Reply-To: squeak at cs.uiuc.edu
> <big snip>
> >8. Given the above, I would avoid being dogmatic on this
> >issue.  I think the fact that so many good Smalltalk programmers
> >still access instance variables directly, and that code that commits
> >this "sin" has lasted unchanged for so long, should cause one to
> >at least question whether or not it's all that sinful.
> <snip>
> 
> Thanks for the brain dump Alan!  :-)
> 
> I understand the pragmatics of this issue well, but I feel compelled to make 
> a counterpoint about the above.  Code that accesses ivars directly can be 
> brittle.  In particular, code which uses both direct access and accessor 
> methods on the same ivars can behave in unpredictable ways.  Sometimes such 
> code survives for a very long time because it is brittle and hard to 
> understand, and no one wants to touch it for fear of breaking it.
> 
> I tend to defer writing code which accesses ivars directly until performance 
> tuning time.  I wish I never had to write such code because it hurt the 
> maintainability and extensibility of my design, but I live in the real world 
> (oh Bah!).
> 
> Thanks again!
> 
> -Carl

Poorly designed code will be brittle.  But I am convinced that blaming 
direct iVar access as the culprit is an oversimplification.

The programmer needs to have a firm grasp of the strategy required for
each variable.  Here are the heuristics I use (or at least, my probably
imperfect and incomplete verbal exposition of them):

	* If there are dependencies on the value of variable v1, such that 
	changes to the value of v1 require consequential action, then one
	should not assign values to v1 directly, but should use a mutation
	message.  

	* If a variable needs to be lazily initialized (perhaps because its
	value is computed by formula, or because its value should only
	be created when needed), then one should normally get the value
	using an access message.  But not always: #release, #postCopy,
	#initialize (this one is an especially interesting case, I won't go
	into it here), #unbindFromV1, #bindToV1, #invalidateV1 and
	#basicV1 are just some of the cases that come to mind where
	one might need to access v1 directly.

	* There are cases where accessor methods don't actually answer
	the value of an instance variable, but answer a copy of its value
	(to prevent unauthorized/stealth changes by outsiders to the value 
	of the object held by the variable).   If the variable really needs
	such protection, then perhaps there should not even be a private
	method that returns it (e.g., #basicV1).  In any case, there would
	be many cases where using the public "accessor" would not
	be approriate for methods internal to the class.  Note that 
	indiscriminately using access messages for all instance variables
	will lead to interesting results whenver an accessor is changed
	to a return a copy of the variable's value.  One needs to consider
	early in the design whether or not the public accessor will
	answer the variable's value itself, or only a copy thereof.

Then there is the infamous case of what I call "fraudulent conversion."
Here's an example: the instance variable is "filename."  The selector
of the accessor is #filename.  The value of the variable is (for example)
a Filename.  The accessor method answers "filename asString." (You
can invert the types, it's still "fraudulent conversion.")  Don't do this.
Instead, have accessors with selectors #filename and #filenameString
(for example).  One always answers the String, the other always
answers the Filename.  If the instance variable always contains
a String, it should be called "filenameString."  And don't let it
contain a String one moment, and a Filename the next.  Of course,
you don't need to have both #filename and #filenameString as
accessor methods if you uniformly deal only in Filenames, or
only in Strings (with respect to the instance variable and its

accessor methods).  This simply eliminates one common need
for using an accessor method (type conversion), because it's
a bad idea in the first place.  Do the type conversion when the
value of the variable is set by the mutator, or else don't name
the type coverting accessor the same as the instance variable.

The other requirement is to use abstract classes whenever and
wherever they make sense, as in the AbstractPoint/CartesianPoint/
PolarPoint case discussed previously.  Code as many methods
as possible in the abstract class, where there are no instance
variables and inappropriate instVar access cannot occur.  The
methods that remain are the ones that probably need to access
the instance variables.

Applying these heuristics results in code that will not be brittle 
due to bad instance variable usage, or so I have found in my own 
experience.

Note that the only strong case for using access messages instead
of reading the value of an instance variable directly involves
lazy initialization (assuming one has used the abstract class/
abstract method pattern appropriately).  If it would just not 
make sense to lazily initialize a variable, then why use an 
access message to get its value?  Accessing the instance
variable directly has a guaranteed semantics, does not require
the reader to browse the accessor method(s) to see what
will really happen, and will not break when the accessor
method is changed to answer a copy or type conversion
of the value of the instance variable. And it's faster, too.

Mutator methods are good because they make it easy to enforce
value constraints, ensure invariants, and take any consequential
action that may be required by a change in the value of a variable.
On the other hand, they don't have guaranted semantics, and so
require the reader to browse the code to see what the mutator
method(s) actually does (do).  It has happened more than once
that mutator methods in subclasses (that override a mutator method
in a superclass) either forget to call "super," or change the semantics 
in such a way that the code in the superclass breaks.  And they
may make it too easy for outsiders to mess with the internal state
of an object.  And they are slower. These things are two edged, 
and whether it is best to use them is very much a judgement call.

--Alan





More information about the Squeak-dev mailing list