[FIX][KCP] KCP-0112-FixCanUnderstand [long!]

Peter van Rooijen peter at vanrooijen.com
Mon Dec 15 15:55:15 UTC 2003


From: "Nathanael Schärli" <n.schaerli at gmx.net>
> Hi guys,

Hi Nathanael!

> > Are you sure this is wise? How exactly did you decide what is "the
> > right way"?
>
> I decided that this is the right way because it is the semantics that is
> expected by the vast majority of senders in the system.

This bothers me. I'm all for democracy, but quite unconvinced that it should
have this type of effect in programming. Especially in a system that is
extensible, where alternatives can be made available quite easily to suit
various people's tastes.

> As an example,
> consider the method ImageReadWriter>>close:
>
> close
> "close if you can"
> (stream respondsTo: #close) ifTrue: [
> stream closed ifFalse: [stream close]]

Maybe that is a good example. Let's assume it is not a hack (in the sense of
code written just to make the thing not conflict with its adopted context -
which it most likely is). Then, what would be a better implementation? What
would be "the best" implementation? The "correct" implementation?

I'm going to look at alternatives without studying the system context, just
looking at this code and what it communicates to the reader, by itself.

But first some questions:

1) The comment is a bit strange. It doesn't express the same as the method
name. The "if you can" part is key. What does the comment actually mean?

2) Could stream possibly be nil? We can't tell from the method itself
because UndefinedObject probably doesn't implement close, so this code could
be functional (work in practice) if stream is indeed nil.

3) The code expects that any object that responds to close, responds to
closed as well. Clearly, the test is an interface/type test. Is there a
Smalltalk pattern for interface/type tests? Perhaps the answer is that there
is not.

4) The code only sends close when stream is not closed. Is this to avoid
unnecessary work? If so, it implies an expectation that the semantics of
stream close doesn't handle streams that are closed in a good manner.

Now some code alternatives:

1)
close
stream ifRespondsPerform: #close

2)
close
stream ifNotNilDo: [:s | s ifRespondsPerform: #close]

3)
close
stream ifImplementsPerform: #close
"implements means responds and is not a marker method doing something like
self error: or self subclassResponsibility or self shouldNotImplement"

4)
close
stream ifNotNilDo: [:s | s ifImplementsPerform: #close]

5)
close
stream close

6)
close
self stream close

7)
closeIfYouCan
stream ifNotNilDo: [:s | s ifImplementsPerform: #close]

And I could go on (and I didn't even do a closed test ;-)).

Maybe my point is that I do agree with you that considering usage is
important:

1) After all the reason for methods existing is them being used. (I say this
here even though it isn't true to make a point also: the marker methods
don't have that purpose - their purpose is to express type info inside a
derivation structure that is based on classes, not types).

2) We should then always keep in mind the question whether we consider the
usage pattern desirable.

> This method uses the message #respondsTo: (which is essentially the same
> as #canUnderstand:) in order to know whether the class of 'stream'
> acually offers the functionality #close and not whether the class of
> 'stream' declares an abstract or disabled method #close!

[I don't agree that the code tests if stream's class understands close. If
it did, it would have sent stream class canUnderstand: #close. But it sends
stream respondsTo: #close, which means that it recognizes the possibility
that stream responds to more than what its class implements as methods,
because respondsTo: may have been redefined for stream.]

This is the type/class confusion again. Classic Smalltalk doesn't have a
type lattice separated from the class hierarchy. Because of this, we can
only add rudimentary/embryonic/partial type info to the system. Should we
expect that this half-baked support for types lets us express a lot of
reasonable type info in an elegant and consistent way? Perhaps we should
simply not expect that.

It seems to me this whole discussion revolves around type/class separation
and type/class confusion. The kind of behavior you are seeking has to do
with types, but you expect/want regular classes to give it to you. I believe
that you will always find conflicting forces if you do that.

> Therefore, the code immediately breaks if it is used for a stream class
> that either declares the method #close to be abstract (i.e., by
> implementing it with the body 'self subclassResponsibility')

I never understood this part of the argument. If we run into a self
subclassResponsibility method, then the implementation of the object we sent
the message to, is already incorrect, right? Why should we expect to be able
to handle it well further down the line?

> or declares
> it to be not appropriate (i.e., by implementing it with the body 'self
> shouldNotImplement').

This is type-related information stuffed in a class hierarchy. Square peg,
round hole. You shouldn't expect it to work elegantly, IMO.

> This is really problematic because especially declaring a method as
> abstract is an extremely important concept of OO programming and it is
> therefore not acceptable that Smalltalk does not offer a programmer to
> do so without breaking the system.

You are indicting Smalltalk for not having a type system like you would want
it to have. You are assuming it has it, but it's broken. I say that it
doesn't have it, so you shouldn't expect it to work.

> I was running in exactly those problems when I wrote an analyzer that
> detects methods that are required in a class (e.g., are self-sent) and
> then automatically declares them as abstract by compiling a method with
> the body 'self subclassResponsibility'. However, this horribly crashed
> the whole image. Why? Because the current semantics of #canUnderstand:
> (and #respondsTo:) does not correspond to the semantics that the users
> of these selectors expect.

Type/class confusion again.

> In fact, there were dozens of places deep in the core of the system
> (e.g., in the Morphic framework) that caused a runtime error just
> because I was being a good programmer and actually declared abstract
> methods as 'subclassResponsibility', which is unarguably a good style of
> programming.

Actually, it is arguable that it is only good style if you accept its
limitations.

> (It makes the code and the used design patterns (e.g., the
> template method pattern) much easier to understand and decreases the
> probability for errors in subclasses).

I do agree with that.

[Note: personally, I write

methodPattern
self halt "subclass responsibility"

because this is portable (subclassResponsibility is not) and halts are
resumable on all dialects, so we get the chance to create a solution in the
debugger.]

> > How do other dialects implement #canUnderstand:? Are they also
> > "wrong"?
>
> I don't know. If they do it the same way as it has been done in Squeak,
> it is more than likely that also in these dialects, the semantics of
> #canUnderstand: and #respondsTo: does not correpsond to what is expected
> in practically all the users of these methods. As a consequence, I
> assume that also these dialects do not allow a programmer to declare
> methods that should be implemented in subclasses as
> 'subclassResponsibility' without breaking other code.

Could you explain again what other code would be broken, and why this code
should work?

> > If you need the new semantics for something you are working on, why
> > not add another method that does exactly what you want instead of
> > modifying this old-timer?
>
> Our goal is to have a kernel that allows a programmer to explicitly
> declare methods that should be implemented in a subclasses (i.e.,
> abstract methods) and methods that are not appropriate for a certain
> class without crashing the whole image.

I don't get it. Especially the "crashing the whole image" part.

> Of course, this could also be done by introducing a new method #foo:
> rather than changing this old-timers. However, this would mean that we
> also had to change 99% of all the callers of these methods so that they
> would use #foo: instead.

If these methods are "wrong" for some reasonable definition of "wrong", then
maybe they deserve to be changed?

> Otherwise, this does not bring us any closer to
> our goal of having a language with a kernel that actually allows
> declaring abstract methods without nasty side-effects!

I basically agree with the goal. I don't agree with your "solution".

> Thus, we have three different choices:
>
> a) Leave everything as it is. This means that declaring a method as
> abstract or inappropriate for a class has nasty side-effects that break
> other code or even crash the image. As a consequence, it is for example
> not possible for a proigrammer to consistently declare abstract methods.
> In fact, in the current version of Squeak I would even suggest a
> programmer never to declare 'subclassResponsibility' method because it
> is safer.

:-). I already don't advocate using subclassResponsibility, see above.

> b) Change #canUnderstand: (and #respondsTo:) so that the semantics
> actually corresponds to what 99% of the the *current* users expect when
> they call it.

Are you saying these (99%) methods are all broken because they expect the
wrong semantics?

> c) Introduce new methods for #canUnderstand: and #respondsTo: and change
> practically all the users of #canUnderstand: and #respondsTo: so that
> they now use these new methods instead.

d) introduce #implements: and #implementsForInstances: which do what you
want (exclude pseudo/marker "implementations"), and then let the maintainers
of the 99% decide if they want to migrate to those, at a time of their
choosing.

> For us, a) is not acceptable because we do not want to have a kernel
> that does not allow clean OO programming. When we had to choose between
> b) and c) we decided for b) just because #canUnderstand: and
> #respondsTo: are such old-timers!! This may sound paradox but it is in
> fact logical:

I don't find you "logic" at all convincing.

> As the Squeak image shows, the vast majority of programmers are used to
> using these old-timer methods when they actually should use the new
> methods. Therefore, when going for alternative c), it would be just a
> question of time until there is new code that inappropriately uses these
> methods again! This is not the case in b), because we make the semantics
> of these methods consistent with what the majority of Smalltalkers
> obviously expected for decades.

The frequent use of "obviously" and variations, doesn't strengthen your
argument, to my mind at least.

> > Obviously this depends on where #canUnderstand: is being called, but
> > the appropriate behaviour to me seems to be to check whether the
> object
> > responds to the message, not to consider what it is going to do *when*
>
> > it responds to the message - that's a slippery slope that we can't go
> > very far down unless we're going to somehow be able to come to a
> > semantic understanding of all arbitrary code.
>
> We neither need nor want to understand the semantics of arbitrary code.
> We just want to offer the programmer a the means to declare a method as
> abstract or not appropriate in a way that is compatible and
> understandable by the meta-protocol of the language.

That sounds quite reasonable. If that is what you want, and Smalltalk does
not offer it in its current state, why don't you *add* it to the language?

So you can write:

abstractMethodPattern
self abstract

and

inappropriateMethodPattern
self inappropriate

Quite intention revealing, I would think, and it doesn't break anything.

> The crux of the problem is that Smalltalk does not have *language*
> feature for these two things. Instead, Smalltalk suggests an idom, which
> is declaring an abstract methods by implementing it with the body 'self
> subclassResponsibility' and declaring inappropriate methods by
> implementing it with the body 'self shouldNotImplement'.

Yes.

> And even though this may seem like a simple and good solution at a first
> glance, it introduces a lot of problems because this way of declaring
> abstract methods is not understood by the meta-protocol of the Smalltalk
> language. As a consequence, all the meta-functionality of Smalltalk
> (i.e., methods such as #canUnderstand: and #respondsTo:) thinks that
> such abstract or disabled methods are regular methods that are
> intended/expected to be called.

No. The meta-functionality does not concern itself with the question if a
method is a regular method or a marker method expressing type information,
because the meta-functionality does not deal with types. You *want* it to
deal with types to a certain extent, but it does not.

> But this is of course the absolute
> *opposite* of what the programmer wants to express and it therefore
> leads to paradox behavior when such meta-functionality is used.
>
> Come to think of it: Isn't it paradox if a programmer wants to
> explicitly state that a method is not implemented in a class (e.g., by
> using 'subclassResponsibility' or 'shouldNotImplement'), and as a
> consequence, all the meta-functionality of Squeak thinks that it
> actually is implemented?

I understand your point. But don't be fooled by the names. Both
implementations are variations of self
thisMethodShouldNotBeActivatedDuringNonDevelopmentOperation.

> And this is precisely what we want to fix: We want to make sure that the
> meta-functionality of Squeak actually understands the special meaning of
> these methods so that the programmer has a way of consistently declaring
> abstract and inappropriate methods. This is all, and it does not require
> understanding arbitrary code. In fact, it just requires understanding
> two idoms (i.e., 'subclassResponsibility' and 'shouldNotImplement').

I still don't understand why you feel you need this change. Perhaps I just
haven't paid attention well enough.

> > I think there is another way of looking at this that might be helpful.
>
> > Let us look at #canUnderstand: as a companion of #doesNotUnderstand:.
> > It doesn't seem so far-fetched to think that "Understand" in both
> > selectors has a related meaning (even though one is on the class and
> > the other on the instance, so there is some inconsistency there).
>
> On a low and technical level, you are right. It is indeed true that
> Squeak 'understands' a message that has been declared as abstract or
> disabled by the programmer by implementing a method with the body
> 'shouldNotImplement' or 'subclassResponsibility'.

Note that you are expecting some kind of fixed/constrained semantics for
these methods. That might not be entirely justified.

> However, this is a
> consequence of the unfortunate fact that Smalltalk language is too poor
> to express declaring abstract or disabled methods in any other way than
> actually implementing them!

Indeed, classic Smalltalk is too poor for that. It is not clear to me if you
are accepting of that fact or not.

> Which is completely paradox to begin with!

Why is that paradoxical? Smalltalk is "too poor" to offer it directly, so
any "support" for it that it does have, is a kludge. And kludges can't be
expected to scale. That is how I see it.

> On a higher and conceptual level, writing a method with the body
> 'shouldNotImplement' or 'subclassResponsibility' is the way a programmer
> declares a method as not appropriate or available in a certain class.
> And therefore, it makes total sense if such a method is then 'not
> understood' by the class. In fact, this is the way it should be because
> it is precisely what the programmer intended to achieve.

You are making an awful lot of assumptions about other programmers'
intentions. Even though I broadly agree with your goals, I feel the concept
of asking for a change to something that has worked for ages, where you
could reasonably use the extensibility of the system to achieve what you
want, is a dubious proposition.

You could choose to look at this issue in this fashion: You have identified
a gap in the meta-protocol. This gives you the opportunity to define new
meta-protocol that others might like and wish to see incorporated in their
systems. Wouldn't that be nice?

> Since I prefer naming methods according to their conceptual and
> higher-level meanig rather than their lower-level implementation (e.g.,
> I prefer #addMethod: rather than #insertMethodIntoMethodDictionary:), I
> prefer the higher-level view. This is particularly the case since we are
> dealing with a higher-level language like Smalltalk and not with C or
> C++.

I agree there. See self abstract and self inappropriate.

> Also, I argue that this higher-level view is the only consistent view
> anyway. Just consider that Smalltalkers implement a method with the body
> 'self shouldNotImplement' to declare that the same method should not be
> implemented! Looking at it on a technical level, this does not make any
> sense, because it is necessary to implement a method in order to express
> that it actually should not be implemented!!

We've covered this. It is the class/type confusion issue. The only way to
really cleanly get rid of it is to implement a type lattice separate from
the class hierarchy. Which, if I'm not mistaken, is not at all inconsistent
with your goals in the Traits proposal. Am I mistaken?

> It is only when we look at this on a higher-level that it makes sense.
> On this level, we just consider the intention of the programmer, and
> this is that he wants to declare that a method is not appropriate for a
> certain class. Whether we do this by associating a designated method
> body to the selector in the method dictionary, or whether we use
> syntactic construct such as in Eiffel (i.e., the 'remove:' clause) does
> not matter.
>
> The only thing that matters is that the programmer knows how to declare
> this and that the system behaves in way that is consistent with the
> conceptual intention of the programmer. Looking at the existing users of
> #canUnderstand: and #respondsTo:, this was obviosuly not the case in
> Squeak. And that's why we suggested this fix.

You still haven't made the point that there is an error in a convincing
manner, so talking about a "fix" still seems to me to be highly prejudicial.

Regards,

Peter




More information about the Squeak-dev mailing list