Mantis Bugtracker
Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ View Advanced ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0001711 [Squeak] Collections minor always 08-18-05 19:04 12-17-08 03:57
Reporter KenCausey View Status public  
Assigned To
Priority normal Resolution open  
Status feedback   Product Version
Summary 0001711: [FIX] Re: Refactoring Associations
Description "Andrew P. Black" <black@cse.ogi.edu>:
Andreas Raab" <andreas.raab@gmx.de> writes:

>To understand the effect have a look at the implementations of
>WeakKeyAssociation>>key and WeakKeyAssociation>>key: - this should make it
>clear that WeakKeyAssociations store their key in form of a weak array so
>that it can be garbage collected. Thus there isn't anything broken, except
>from the interpretation of the key iVar.

I figured out what was going wrong - one last WeakKeyAssociation
object of the old kind (with the array) was being created by the act
of patching in the new kind of WeakKeyAssociation. (Changing the
class hierarchy created an obsolete subclass, which had to be weakly
remembered...)

However, by the time that this errant object was created, the method
dictionary of its class had been zapped
(Behavior>>canZapMethodDictionary always returns true; perhaps it
should check to see if there are still any instances or the potential
to create any). Thus, this object didn't behave correctly, because
its key method was the one inherited from LookUpKey.

By doing a double renaming I at last have a change set for this
refactoring that can be loaded, at least, it loads fine into a clean
3.7beta #5969.

This changeset passes the tests that I posted a couple of weeks ago."
"
Additional Information
Attached Files  AssociationRefact.Hand.6.cs.gz [^] (2,961 bytes) 08-18-05 19:05
 AssociationRefact.cs.gz [^] (3,113 bytes) 08-18-05 19:08
 AssociationRefactTests.cs.gz [^] (1,802 bytes) 08-18-05 19:08
 AsssociationRefactCleanUp.cs.gz [^] (957 bytes) 08-18-05 19:08

- Relationships
related to 0001722feedback  [ENH] Associations Please review and read the tests 

- Notes
(0002417 - 224 - 258 - 258 - 258 - 258 - 258)
KenCausey
08-18-05 19:05

 karl.ramberg@chello.se:

"Hi, Andrew. I'm not sure what this changeset does. Could you check it
against the 3.9alpha if there are any conflicts, and give a short
description in the preamble of what the change set does ?"
 
(0002418 - 1939 - 2157 - 2157 - 2157 - 2157 - 2157)
KenCausey
08-18-05 19:07

 tomkoenig@mindspring.com:

"Tested with 3.9a#6506 including execution of the sunit tests Andrew
provided in a separated email.
Reviewed all code for quality and conflicts. The refactoring is not
crucial but it definitely makes these core classes cleaner. The work on
cleaning up the code, creating a usfule test suite for this important
set of classes and figuring out how to install the change definitely
merits inclusion in 3.9a. (There is no particular need to add it to
3.8b, although I did some of my orignal testing there.)
Background from preamble:
" tlk:Can you tell me something about the motivation for the original
Association Refactoring cs ?
ab: Essentially, the problem was that several methods (like printOn: !)
were missing from some subclasses of Association, like
ReadOnlyVariableBinding. Also, WeakKeyAssociations were a really ugly
hack. The solution seemed to be to refactor the class hierarchy. This
was not hard, but coming up with a changeset that would file in without
breaking the compiler half way through was a challenge!
This changeset was produced by hand. The order in which the various
classes are defined or redefined is critical."

AssociationRefact is the prereq for the other two cs's. It is Andrew's
original cs with a change to the preamble to explain the motivation.
AssociationRefactTest is tests the first cs and is a prereq for the
final cs. Andrew sent me this cs, all I did was rename it, add a
preamble and add a class comment for the test class. (I know, anal).
AssociationRefactCleanUp is my work. It includes class comments for the
association classes and a postscript to remove a now redundant test
class. You could accept the first cs, or the first two cs and
everything would be okay. But I think all three together make the
better package."

(attaching AssociationRefact.cs.gz, AssociationRefactTests.cs.gz, AsssociationRefactCleanUp.cs.gz)
 
(0002419 - 286 - 332 - 332 - 332 - 332 - 332)
KenCausey
08-18-05 19:08

 ducasse@iam.unibe.ch:

"Pay attention this is a test to see if we can get experts looking at
changes.
So can someone with knowledge on association and compiler have a look at
this refactoring. I know that andrew spent a lot of time on it and I
would like to avoid to see it lost."
 
(0002420 - 217 - 217 - 217 - 217 - 217 - 217)
KenCausey
08-18-05 19:10

 I was able to load the original changeset AssociationRefact.Hand.6.cs.gz into 3.8-6665-basic without errors but I did not check it further. Whenever I attempted to load any of the other 3 changesets I got a debugger.
 
(0012855 - 216 - 228 - 228 - 228 - 228 - 228)
Keith_Hodges
12-17-08 03:37
edited on: 12-17-08 03:57

 3.11 Feedback - Need to review the code relative to 3.10.2bc. We have atomic loading for change sets. Name the change set .mcs so that monticello will load it, and load with Installer (with atomic loading enabled).
 

- Issue History
Date Modified Username Field Change
08-18-05 19:04 KenCausey New Issue
08-18-05 19:04 KenCausey File Added: AsssociationRefactCleanUp.cs.gz
08-18-05 19:05 KenCausey File Deleted: AsssociationRefactCleanUp.cs.gz
08-18-05 19:05 KenCausey File Added: AssociationRefact.Hand.6.cs.gz
08-18-05 19:05 KenCausey Note Added: 0002417
08-18-05 19:07 KenCausey Note Added: 0002418
08-18-05 19:08 KenCausey File Added: AssociationRefact.cs.gz
08-18-05 19:08 KenCausey File Added: AssociationRefactTests.cs.gz
08-18-05 19:08 KenCausey File Added: AsssociationRefactCleanUp.cs.gz
08-18-05 19:08 KenCausey Note Added: 0002419
08-18-05 19:10 KenCausey Note Added: 0002420
08-22-05 18:29 KenCausey Relationship added related to 0001722
12-17-08 03:37 Keith_Hodges Note Added: 0012855
12-17-08 03:57 Keith_Hodges Note Edited: 0012855
12-17-08 03:57 Keith_Hodges Status new => feedback

Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
70 total queries executed.
39 unique queries executed.
Powered by Mantis Bugtracker