Hi Boris,

> I feel sorry that I needed three attempts and four mails to
> come up with a reasonably good proposal, but the stuff
> is really a bit tricky. Comments are therefore most welcome.

Only one comment: Since this stuff is indeed tricky, I would very much
appreciate a test which documents the problem you've noticed. Having a test
for each bug found will ensure we avoid it in future changes.

Cheers,
  - Andreas

----- Original Message ----- 
From: "Boris Gaertner" <Boris.Gaertner at gmx.net>
To: "The general-purpose Squeak developers list"
<squeak-dev at lists.squeakfoundation.org>
Sent: Monday, January 05, 2004 9:04 PM
Subject: Re: [BUG] ClassBuilder cannot change class to variable subclass


> From: ducasse <ducasse at iam.unibe.ch>
>
>
> > Hi boris
> >
> > When we started KCP, we were looking at withAllSubclassesDo: we realise
> > that the order of iterations
> > could be really important. This is a non-functional requirement that is
> > fun but dangerous.
> > I did not have the time to look at your changes now but I'm guessing
> > that the order is really important.
> > Is my hypothesis true?
> I think it is not.
>
> I compared earlier versions of the ClassBuilder with the current one
> and I think now, that both my proposals to fix the bug are bad.
> My first proposal is wrong and my second one is bad because it
> does unnecessary tests.  Let me explain this in greater detail.
>
> As I wrote last night (Jan 04), an earlier version of
> ClassBuilder>>validateSubclassFormat:from:forSuper:extra:
> used two iteration statements to keep two tests separate.
> This is a very important point, but yesterday I failed to
> fully understand it.
>
> The first test checks the number of named instance variables.
> This test is needed for the modified class and all its subclasses.
> For this test the use of "withAllSubclassesDo:" is correct
> and the order of the tests is irrelevant.
>
> The second test verifies that, with the new format, oldClass
> is general enough to be the superclass of its subclasses.
> Here it is sufficient to check the direct subclasses. This
> is also said in the comment that survived the changes that
> came with change set 5300:
>  "And check if the immediate subclasses of
> oldClass can keep its layout"
> Here the use of "subclassesDo:" is sufficient and "subclassesDo:"
> iterates over a collection that is often smaller than the collection
> that "allSubclassesDo:" iterates over.
> (It is not necessary to check the subclasses of the subclasses
> because this was done for every subclass when it was defined
> or modified for the last time.)
>
> I think that we should go back to the use of two iteration statements.
> The attached third proposal of a fix supersedes my two earlier
> proposals. (The proposal was of course checked against the
> tests that Andreas posted yesterday. Thank you, Andreas!)
>
> I feel sorry that I needed three attempts and four mails to
> come up with a reasonably good proposal, but the stuff
> is really a bit tricky. Comments are therefore most welcome.
>
> Greetings, Boris
>


----------------------------------------------------------------------------
----


>
>