ClassBuilder changes

Daniel Vainsencher daniel.vainsencher at gmail.com
Mon Oct 30 13:34:28 UTC 2006


Hi everyone,

I wanted to note that Andrew Black and I have also touched the change 
notification integration into class builder last summer during the 
Traits work.

The story as I remember it - the traits work required massive recreation 
of classes due to adding variables somewhere around behavior. It turned 
out that this corrupted the subclasses/superclass relationship, and we 
traced the bug to the code you're looking at. If I remember correctly, 
the problem was that for SCN, CB creates a copy of the class, but this 
was not always cleaned up because CB uses an early return to signal 
failure. IIRC, my fix was to add the send of #ensure: you can see is now 
there.

I didn't, however, add the copying of the class itself - that was 
already there.

To move forward - anyone else been touching that code, and possibly 
remembers what the copies were added for? AFAICT, 
ModifiedClassDefinitionEvent does indeed keep copies of the new and old 
definitions, and from looking at their uses, I think Andreas' suggestion 
of a "record object" would probably do just fine. At a second look, I 
would guess that the use of copies of the old class dates to the 
addition of the "anyChanges" method, apparently by NathanaelS.

Daniel Vainsencher


Roel Wuyts wrote:
> Hi Andreas,
>
> that part of the code was not me, I think.
>
> For the change notification I only asumed to get the *name* of a 
> renamed or removed element to be passed around (whether a class or a 
> method or whatever else), not the removed/renamed element itself (for 
> the reasons you mention). But the version in the image is not exactly 
> mine (whoever put it in there made some changes for better or for 
> worse), and this version does not seem to be from me. But ok...
>
> I can fix this, I guess (I think that Nathanael was the maintainer for 
> the class builder, but I do not know whether he is still around for 
> this)...
>
> You need this urgently, I assume ?
>
> PS: Can you reply personally to me as well ? I do not follow the 
> squeak list as often as I should :-( But I do fix things that get 
> included in the releases :-)
>
>
> On 24 Oct 2006, at 10:09, Andreas Raab wrote:
>
>> Hi Roel -
>>
>> Roel Wuyts wrote:
>>> long time no see :-) rw is me.
>>
>> Ah yes, should have thought of that.
>>
>>> The implementation checks for these occurences and is validated by 
>>> the corresponding unit tests.
>>> Do you think there is problems with it?
>>
>> I am suspecting there is an issue with traits registration due to the 
>> copy of the original class you are creating in ClassBuilder. Which 
>> seems to be required by the notification mechanism but strikes me as 
>> conflicting by definition, since it asks for both, the old *and* the 
>> new class to be accessible at the same time. Which, by virtue of the 
>> invariants guaranteed in ClassBuilder, should never ever happen (and 
>> I am VERY suspicious of just throwing incomplete copies of classes 
>> around).
>>
>> Question: Wouldn't it be more logical, less conflicting and just 
>> generally better design to drop the requirement of having both 
>> classes accessible and instead simply use a ClassDefinition object 
>> for the old class? This may look a lot like a class except that it 
>> isn't a behavior and therefore not subject to ClassBuilder invariants.
>>
>> Cheers,
>>   - Andreas
>
>





More information about the Squeak-dev mailing list