On Mar 16, 2010, at 9:50 PM, Lukas Renggli wrote:

> I have some comments, to both the package implementation and the
> Glamour browser. The other browser didn't work, I guess I miss some
> extra package.
> 
> - I used the code below to import the existing packages into the new
> model. Maybe something like this should be included on the class side
> of RPackageOrganizer to have a realistic model?
> 
>       | package |
>       PackageOrganizer default packages
>               do: [ :info |
>                       package := RPackage2 named: info packageName.
>                       info classes do: [ :each | package addClassDefinition: 
> each ].
>                       info coreMethods do: [ :each |
>                               each isValid
>                                       ifTrue: [ package addMethod: each 
> compiledMethod isExtension: false ] ].
>                       info extensionMethods do: [ :each |
>                               each isValid
>                                       ifTrue: [ package addMethod: each 
> compiledMethod isExtension: true ] ] ]
>               displayingProgress: 'Importing'
> 
> Then I opened the glamour browser using:
> 
>       GTCoder openOn: RPackageOrganizer default
> 
> - I find it quite strange that I have to declare if a method is an
> extension or not. Isn't a that obvious when looking at the defined
> classes?

Historically it was like you said in implementation 1 and 2, then I removed the 
defined classes (implementation 3), then  I realized oops shit, we can have 
package with class but no methods (and in this case
we should always when removing a method check if this is the last one and if 
there is none in the metaclass
to trigger a classremoved event), so I reintroduced defined classes as a 
separate data (implementation 4 = now).
Now I think that we should remove isExtension to go back to the first 
implementation.
I guess that this is a todo

> Having two dictionaries for methods makes it extremely
> difficult to move stuff around because always 4 separate cases need to
> be handled.

I discussed that point with doru when you went off your office. 

> 
> - The fact that compiled methods are stored in the model is very
> dangerous.

There should not be or doru changes my code!!!

> You might hold onto compiled methods that have long been
> replaced with other ones. Just by playing a bit with the model I run
> into that situation (I don't know how). I think just keeping the
> selector internally would be much safer and solve all kinds of
> troubles (exactly like this is done for the classes).

but this is like that.
Doru?

> You'll have to
> check anyway if the method is still present when you iterate over the
> methods. A single use of #doSilently: (and there are lots of them in
> the image) can completely screw up the complete package model.
> 
> - The browser displays nicely the extended classes, but for the
> methods I don't see the protocols and the complete set of selectors
> implemented. I think these things should be part of the browser,
> otherwise we don't see if the package model can answer these queries
> quick enough.
> 
> - The browser displays extension methods on both instance and
> class-side. When browsing an extended class, the extensions are not
> displayed.
> 
> That's it for the moment. I see a cool model emerging :-)
> 
> Lukas
> 
> -- 
> Lukas Renggli
> http://www.lukas-renggli.ch
> 
> _______________________________________________
> Pharo-project mailing list
> Pharo-project@lists.gforge.inria.fr
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
Pharo-project@lists.gforge.inria.fr
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Reply via email to