Hi benjamin

Thanks for your code!!!!

I did a review of your latest code: Here are some points that it would be nice to address I fixed what I saw and republished

0 - all the classes should have comments

I'm on it, but it's a bit long

MessageList ---------- 1- MessageList example does not work Can you create some Unit tests to cover the expected behavior of the MessageList

I've fixed it thanks to Guillermo Polito :)

2- MessageList>>browsers I removed it

Thank you

3- recategorized the methods

4- I removed all the "comment10"

Thanks again

5- what I level Can you comment it?

It's commented now. Level represents the number of levels used to shows the list

6- for asDictionaryByClass why don't you use groupedBy: or similar methods? I'm not sure that I would call them asDictionaryBy.... since you return an association?

Why don't you call them groupedBySelectors, groupedByClasses, groupedByPackages then why do you return an Association and not a dictionary.

I've changed the names. They return an association because they return the dictionary and the list sorted. I've choose this way because when i add referecens in the dictionary, the references are sorted alphabetically and so I lost the order :s

7- Can you add oneline method comments to method?

I'm on o too

8- Could you introduce an explicit instance variable representing the environment in use and initialize it with Smalltalk globals because like that we can specify in which environment the set is working.

I do not understand ...

9- openInWorldWithADictionarySelector: sucks :) the name is not really cool then you bind the core with the UI so you should define this method as an extension of RecentSubmission-UI and may be

I've removed this method

10- You update all the browsers open but this should not be your role. As a core domain object you should not do this kind of behavior. The UI widget should register to event or notification and update themselves when you change.

updateView browsers ifNotNilDo: [:collection | collection do: [:each | each updateView]]

I've fixed that using Announcement MessageList >> updateView self triggerEvent: #changed MessageListBrowser >> messageList: anObject messageList := anObject. messageList when: #changed send: #updateView to: self. messageList when: #reset send: #delete to: self.

MessageListAssociation -------------------- why do you need this guy? Just to print the elements? This is a UI concerns. The UI should wrap the model and provide a specific printed version or the MessageListAssociation should be explained in the class comment

I used them to quickly have the MethodReference corresponding to a node.

MethodReferenceWithSource ------------------------ We should probably move some behavior to MethodReference

I do not dare to change Pharo core alone ^^

NullTextStyler ------------ I categorized all the methods following SHStyler categories

Thank you :)

RecentMessageList ---------------- I cleaned the singleton part. You do not need UniqueInstance ifNotNil: [:u | UniqueInstance releaseAll].

Again the model should not be referencing the ui releaseAll super initialize. MessageListBrowser allInstancesDo: [:each | each messageList = self ifTrue:[each delete]]. browser:= nil. self methodReferenceList: OrderedCollection new.

Ok, now i used event too :)

Why the setting is registration is done in the core and not at the level of the widget?

It's just easier to write in RecentMessageList self openInWorld than to write in MessageListBrowser on: RecentMesageList uniquInstance withADictionarySelector: RecentMessageList uniqueInstance sortingSelector

- at startUp: you should not create the instance since uniqueInstance is lazzily accessed. However you should register to the SystemChangeNotifier to get the notification.

It's done, i've implemented the RecentMessageList class >> event: method.

MessageListBrowser ----------------- Instead of having

MessageListBrowser >>byDateAscendingOn: aMessageList

^self on: aMessageList withADictionarySelector: #asDictionaryByDateAscending.

MessageListBrowser >>byDateAscendingOn: aMessageList

^self on: aMessageList withADictionarySelector: aMessageList asDictionaryByDateAscendingSelector

and

MessageList>> asDictionaryByDateAscendingSelector ^ #asDictionaryByDateAscending

=> encapsulate information from model to UI. => I did it

I do not understand :s

So for now this is enough :)

Cool :) Thank you and Guillermo Polito Ben Gofer new squeaksource: 'PharoTaskForces'; package: 'RecentSubmissions'; load.