Trouble with MVP Night School

Hi,

I'm currently reading and trying the code in Ian's first message in the
MVPNightSchool2 thread (see
http://www.object-arts.co.uk/wiki/html/Dolphin/MVPNightSchool2.htm)

The 'books' variable in Library is bound to a SortedCollection. In the
LibraryShell (the presenter) a booksPresenter variable is bound to a
ListPresenter. So far so good.

The LibraryShell>>onLibraryChanged method is defined as:

LibraryShell>>onLibraryChanged
        booksPresenter list: self model books.
        self onLibraryStatusChanged.

This ends up sending the #list: to a SortedList (ListPresenter>>list:
sends a #list message to its model) which doesn't understand it. :-(

What am I doing wrong?

Thsi MVP thing is driving me nuts...

Fernando,

> The 'books' variable in Library is bound to a SortedCollection. In the
> LibraryShell (the presenter) a booksPresenter variable is bound to a
> ListPresenter. So far so good.
>
> The LibraryShell>>onLibraryChanged method is defined as:
>
> LibraryShell>>onLibraryChanged
> booksPresenter list: self model books.
> self onLibraryStatusChanged.

I read the middle line as

    booksPresenter list: self model availableBooks

although it shouldn't make any difference as both methods will answer a
SortedCollection.

> This ends up sending the #list: to a SortedList (ListPresenter>>list:
> sends a #list message to its model) which doesn't understand it. :-(

I'm not absolutely sure what you have done (what's a SortedList?) [1] but
the above line sends #list to booksPresenter, a ListPresenter.  The
ListPresenter>>list: method then fiddles about with it a bit (to sort it if
needed) and then sends #list to it's model.  A ListPresenter's model is an
instance of ListModel , not a SortedCollection, and that implements the
#list: method so shouldn't raise an exception.

If you can't spot the problem copy the classes into a package and post it
here as part of a reply (or mail it directly to me as an attachment if it's
easier) and I'll have a look at it.

[1] One possibility is that somewhere you are evaluating ....

booksPresenter model: self model books

which will have the effect of replacing the ListPresenters model with a
SortedCollection.  I can't see where that might happen though.

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

>I'm not absolutely sure what you have done (what's a SortedList?) [1]
but

A typo. :-) It hsould have been SortedColletion.

>[1] One possibility is that somewhere you are evaluating ....
>booksPresenter model: self model books

Yes, I was evualting it in the LibaryShell>>model: method. I though it
was necessary...

model: aLibrary
        super model: aLibrary.
        booksPresenter model: aLibrary books.
        availableBooksPresenter model: aLibrary availableBooks.

Fernando,

> A typo. :-) It hsould have been SortedColletion.

I guessed that was what you really meant.:-)

> Yes, I was evualting it in the LibaryShell>>model: method. I though it
> was necessary...

Nope.  When you create a ListPresenter (or most any Presenter) it
automatically initializes it's own model - see the method ListPresenter
class>>defaultModel.

You can also see the behavior in a workspace.  "Inspect"

lp := ListPresenter new.
lp model

and you will see that the ListPresenter's model defaults to an, empty,
ListModel.  To put something into the ListModel you send it the #list:
message.  "Inspect"

lp model list: #(1 2 3 4 5).
lp model.  "==> a ListModel"
lp model list  "==> an Array"

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

On Fri, 18 Feb 2005 15:34:56 -0000, "Ian Bartholomew"
<[hidden email]> wrote:

Ian,

On the 1st stage of the MVPNightSchool2 you have this code

LibraryShell>>onLibraryChanged
        booksPresenter list: self model availableBooks.
        self onLibraryStatusChanged.

Shouldn't it be:
booksPresenter list: self model books. ?

The whole list of books has changed, not just the availability of a
book.

I tried changing the code to
booksPresenter list: self model books.

But now everytime I add a book to the Library, only the list of
available books is update on the view. =:-O  What the heck????

Sorry to keep bugging you with this... O:-)
Thanks!

Fernando,

> Shouldn't it be:
> booksPresenter list: self model books. ?

Yes, you are quite right.  I've just worked through the first part and that
was an error in the original.  It was probably missed as it didn't affect
the explanation and the method is removed in subsequent parts.  No excuses
though, it was an error - sorry.

> But now everytime I add a book to the Library, only the list of
> available books is update on the view. =:-O  What the heck????

That's because of a Dolphin feature that you will undoubtedly come across
again.

ListPresenters try to act intelligently and attempt to detect when they are
asked to redisplay a list that they are already displaying.  In that
situation they ignore the replacement request and leave the current list
alone.  This test is done using object identity (two obects are the same)
rather than object equality (the contents of the two objects are the same).
See the method ListModel>>list for the actual code.  This has the following
effect (try this in a workspace)

list := #(1 2 3) asOrderedCollection.
listPresenter := ListPresenter show.
listPresenter list: list

OK, that now shows the list in a presenter.  Now evaluate

list add: 99.
listPresenter list: list

The displayed list does not show 99 as the ListPresenter has decided that as
the list's _identity_ hasn't changed, it's still the same OrderedCollection
as before, then it's contents haven't changed either.  To get around this
you have to change the OrderedCollection's identity, the easiest way is just

listPresenter list: list copy

As it now seen as a new list then the ListPresenter should update it's
display.

Going back to the original problem you had.  Library>>books always answers
the same collection of books, the one held in the Libraries #books instVar.
As this is always the same, identical, object the ListPresenter never
bothers updating it's list.

To fix it you just change the Library>>books method to answer a copy, rather
than the original collection.  This is always a good idea anyway as it
preserves the Libraries encapsulation, nothing else can change the books
collection held by the Library.

> Sorry to keep bugging you with this... O:-)
> Thanks!

No problem, and apologies again for the original error.

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

On Thu, 3 Mar 2005 17:06:52 -0000, "Ian Bartholomew"
<[hidden email]> wrote:


>To fix it you just change the Library>>books method to answer a copy, rather
>than the original collection.  This is always a good idea anyway as it
>preserves the Libraries encapsulation, nothing else can change the books
>collection held by the Library.

Isn't this whole thing a bit wasteful? I mean, sending the whole list,
or copies of it, back and forth everytime a single book is added.

Isn't this expensive?

On Thu, 3 Mar 2005 17:06:52 -0000, "Ian Bartholomew"
<[hidden email]> wrote:



>Going back to the original problem you had.  Library>>books always answers
>the same collection of books, the one held in the Libraries #books instVar.
>As this is always the same, identical, object the ListPresenter never
>bothers updating it's list.
>
>To fix it you just change the Library>>books method to answer a copy, rather
>than the original collection.  This is always a good idea anyway as it
>preserves the Libraries encapsulation, nothing else can change the books
>collection held by the Library.

However, this stops working as soon as I try the 'Stage 2'
modifications. It works if Library>>books storps returning a copy of
the ListModel.

I though #copy meant "shallow copy"... Is it creating a copy of
ListModel with a new SortedCollection in it?

Fernando,

> Isn't this whole thing a bit wasteful? I mean, sending the whole list,
> or copies of it, back and forth everytime a single book is added.
>
> Isn't this expensive?

Not really.  You are either passing an object back (not forth as the
Presenter never tells the Model what books it has) or a copy of the object.
Taking a shallow copy of something is a very common operation in Smalltalk
and doesn't incur much overhead.  It also fits in with what I was trying to
show - the interaction between the Model and Presenter....

-- The Model changes and triggers an event to say the library has changed
-- The Presenter responds to the event and asks the Model for the current
list of books in the library

I could have worked it so that the Model triggered different events ....

#bookAddedToLibrary:
#bookRemovedFromLibrary:
#bookBorrowed:by:
#bookReturned:

... and have the Presenter respond to them individually, but it would have
complicated things somewhat - and it was intended as a /simple/ example of
MVP.

KISS and YAGNI spring to mind :-)

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

Fernando,

> However, this stops working as soon as I try the 'Stage 2'
> modifications. It works if Library>>books storps returning a copy of
> the ListModel.
>
> I though #copy meant "shallow copy"... Is it creating a copy of
> ListModel with a new SortedCollection in it?

Yes, but the whole point of this part is that the Library and the
BooksPresenter are now sharing the /same/ instance of ListModel.  This
removes the need for the Library to inform the BooksPresenter that the
Libraries collection of books has changed.

When we add a book (Library>>addBook:) then the ListModel triggers a
#item:changedAtIndex: event.  If the Library and the BooksPresenter are
sharing the ListModel then both can receive this event, the Library will do
nothing as it hasn't registered for the event but the BooksPresenter has,
and will update it's list.

If the Library returns a copy of the books ListModel it will not then be
sharing the same instance of ListModel with the BooksPresenter.  When we add
a book it will trigger an event off of the Libraries copy of the ListModel
but not the BooksPresenters.  NB a copy of a ListModel does not
automatically copy the list of events/receivers set up in the original. Try
this in a workspace...

lib := ListModel new.
lib when: #item:addedAtIndex: send: #xxx:yyy: to: self.

You can see the event available in lib...

lib events inspect

...but not in a copy of lib

lib copy events inspect

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

On Fri, 4 Mar 2005 16:22:16 -0000, "Ian Bartholomew"
<[hidden email]> wrote:

Yes I understand that part, it's just that I had some misconceptions
on the way #copy works. I'm loking into that right now.

I expected this to return true:

a := #(1 2 3).
b := a copy.

a at: 3 put: 5.
(b at: 3) = 5

and it doesn't...

If anybody knows some good online article or tutorial on this issue,
I'd appreciate it.


Thanks! :-)

Fernando wrote:
Hi Fernando,

being only a Smalltalk newbie myself I'd guess you could probably find
your answers in the methods in protocol "copying" of class Object.

Kind regards,
        Arie

Fernando,

> I expected this to return true:
>
> a := #(1 2 3).
> b := a copy.
>
> a at: 3 put: 5.
> (b at: 3) = 5
>
> and it doesn't...

And that is a good thing.  The you made a copy and then modified the
original collection; the copy should be unaffected by the change.

Just a hunch, are you wondering about shallow and deep copies, or
perhaps trying to share a model among presenters?

Have a good one,

Bill

--
Wilhelm K. Schwab, Ph.D.
[hidden email]

On Fri, 04 Mar 2005 15:51:11 -0500, Bill Schwab
<[hidden email]> wrote:


>And that is a good thing.  The you made a copy and then modified the
>original collection; the copy should be unaffected by the change.
>
>Just a hunch, are you wondering about shallow and deep copies, or
>perhaps trying to share a model among presenters?

The former. I don't know how shallow is shallowCopy.

Fernando,

>>Just a hunch, are you wondering about shallow and deep copies, or
>>perhaps trying to share a model among presenters?
>
>
> The former. I don't know how shallow is shallowCopy.

An ANSI working group would probably arrive at something along the lines
of "not deeper than #copy" ;)  FWIW, that is probably a pretty good
answer =:0

It would probably help you to focus on #postCopy.  My experience has
been that once I understood how to make #copy do what one would expect
for given class (via #postCopy), I had much less need to worry about
shallow and deep copies.  They appear in my code, but only rarely.

When you want a _really_ deep copy, you can put an object through the
binary filer, something like

    aCopy := Object fromBinaryStoreBytes:anObject binaryStoreBytes.

Note that the above will not do silly things like copying classes; STB
proxies prevent problems like that.

However, if you are trying to get a class of yours to copy correctly,
#postCopy will usually do the job.

You asked about recommended reading.  It won't help too much on this
topic, but be sure to grab Simon Lewis' Art and Science of Smalltalk.
IIRC, it is available free as a PDF.

Have a good one,

Bill


--
Wilhelm K. Schwab, Ph.D.
[hidden email]

Bill Schwab wrote:

[...]

> You asked about recommended reading.  It won't help too much on this
> topic, but be sure to grab Simon Lewis' Art and Science of Smalltalk.
> IIRC, it is available free as a PDF.

Yes, it is. I found it at
  http://www.iam.unibe.ch/~ducasse/FreeBooks/Art/artMissing186187Fix1.pdf

BTW there are links to - among other topics on Smalltalk - more free
books at
   http://www.cetus-links.org/oo_smalltalk.html

Best Regards,
Bernhard