Issue 5283 in pharo: Little refactor in PluggaleListMorph

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo
Status: FixReviewNeeded
Owner: [hidden email]
Labels: Type-Cleanup Milestone-1.4 Difficulty-Easy

New issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

And an optimization to prevent to rebuild the whole list twice

Attachments:
        RefactorInPluggableListMorphPart1.1.cs  9.9 KB
        RefactorInPluggableListMorphPart2.1.cs  1.3 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo

Comment #1 on issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

youpi!




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo

Comment #2 on issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

Hi ben

I do not like too much adding methods in Object.

I think that the old logic (cleaning the list is strange to me). But now I  
do not understand how such changes
avoid to rebuild the whole list twice

"Get rid of blanks and style used in some lists"

nextSelection := self getList
        detectIndex: [:a | a beginsWith: lastKeystrokes fromList: self ]

nextSelectionText := self getList
        detect: [:a | a asString trimBoth asLowercase beginsWith: lastKeystrokes]


you removed
"nextSelection := self getList findFirst: [:a | a = nextSelectionText]."
and it seems that this is this that was rebuild it.

So I would like that we discuss.

Stef



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo

Comment #3 on issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

The method on Object could be removed (and moved on Text).

Each time you send getList, the list asks the model for the list and  
rewraps every items. it could be very slow.

In the old code, you do a detect to retrieve the element which match the  
lastKeyStroke (first iteration over the collection) and then do a findFirst  
to retrieve the index. But here it used to call getList (second iteration).

Obviously, you only need the index of the element you are looking for  
(nextSelectionText is not used except for that).

And moreover it could introduce some bug if when you wrap object you return  
a morph by example, then "a = nextSelectionText" will always be false.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo

Comment #4 on issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

Here is a cs file to remove method from Object

Attachments:
        RefactorInPluggableListMorphPart1.2.cs  26.4 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo
In reply to this post by pharo
Issue 5283: Little refactor in PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

This issue is now blocking issue 5288.
See http://code.google.com/p/pharo/issues/detail?id=5288

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5283 in pharo: Little refactor in PluggaleListMorph

pharo
In reply to this post by pharo
Updates:
        Status: Closed

Comment #6 on issue 5283 by [hidden email]: Little refactor in  
PluggaleListMorph
http://code.google.com/p/pharo/issues/detail?id=5283

I used findFirst: and removed the introduced method in collection.
In 14332


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker