MoenTreeView ... um ... challenges

I'm trying to implement a MoenTreeView on a TreeModel.  It seems like it
should work, but it doesn't do what I'd expect and it certainly does not
seem to exhibit behavior similar to a TreeView on a TreeModel.  I've looked
at DSDN and seen some posts but those don't seem to help, so after a week of
deja vu again and again, here's what I've got.

First off, start with a TreePresenter and evaluate each of the following
lines separately:
tp := TreePresenter showOn: TreeModel new.
tp model add: View asChildOf: nil.
tp model add: Object asChildOf: nil.
tp model move: View asChildOf: Object.
tp model move: View asChildOf: nil.
tp model remove: Object.
tp model remove: View.
These seem to work fine, except that after View is moved to be a root
object, Object shows a 'plus' sign to its left, signifying it has children
when in fact it doesn't.  Adding "tp view refreshContents" at that point
fixes it, but it seems like one shouldn't have to do that.

The next step is to try this with a MoenTreeView:
tp := TreePresenter show: 'Moen tree' on: TreeModel new.
tp model add: View asChildOf: nil
Nothing appears in the view.  If you evaluate "tp view refreshContents" at
this point, you get a walkback and you'll see that the node is
uninitialized.  I remember reading that Blair said that tnode clip should
never be nil, but it is -- maybe that's the problem.

Then again, maybe wrapping it in a ScrollingDecorator will help, though it
seems like one shouldn't *have* to do that.  So now try this:
tp := TreePresenter create: 'Moen tree' in: (CompositePresenter show:
'Scrolling container') on: TreeModel new.
tp model add: View asChildOf: nil
tp model add: Object asChildOf: nil
tp model move: View asChildOf: Object
Nothing appears in the view after either of the first two adds and a
walkback appears after the move, saying that View isn't found.  Something's
starting to make a little sense now, i.e., even though the model triggers an
event, the view is not adding the nodes.

Try it again, this time interspersing a #refreshContents message between
almost every line and you get this:
tp := TreePresenter create: 'Moen tree' in: (CompositePresenter show:
'Scrolling container') on: TreeModel new.
tp model add: View asChildOf: nil
tp view refreshContents
tp model add: Object asChildOf: nil
tp view refreshContents
tp model move: View asChildOf: Object
tp view refreshContents
tp model move: View asChildOf: nil
tp view refreshContents
tp model remove: Object
tp model remove: View

I wouldn't be surprised if I'm missing something here (standard brain
disclaimer) but having to send #refreshContents seems rather heavyhanded and
duplicative -- the model triggers add, move, etc events which the view
should pick up on.  It also indicates the MoenTreeView is not as pluggable
as one would think.  I've kept this example brief, but I think it's even
worse when one gets into adding, moving, and removing more than one class at
a time -- I know in some cases even #refreshContents didn't do the job and I
had to send "tp model: tp model" (which really digs into my aesthetic
nerve).  I was hoping to implement, among other things, a graphical browser
that shows a subset of classes.

I'd appreciate help and insight on this.

-- Louis

Louis,

A vague thought on reading your post.

Unlike the Windows TreeView control MoenTreeView is completely implemented
in Smalltalk. If there was an automatic #refreshContents, or any attempt to
redraw the complete view (quite a complex operation by the look of it),
every time an item was added then there would be a big performance hit.
Wouldn't it be better to let the application decide when it was time for the
view to be redrawn?. You can argue that the TreeView control already does
this as well - Windows does not fully redraw every time you add an item to a
TreeView.

I would think that doing additions/updates to the TreeModel and then, when
you are finished, updating the view via #refreshContents would be the best
pattern to follow.

Having said that it does look as if the Dolphin implementation of
MoenTreeView is broken, as your #move example demonstrates, so maybe OA do
intend to allow the view to update after every change and #add is in error.

Just my 0.2 euro

Ian

Louis

"Louis Sumberg" <[hidden email]> wrote in message
news:9a8mf0$44p8m$[hidden email]...
> I'm trying to implement a MoenTreeView on a TreeModel.  It seems like it
> should work, but it doesn't do what I'd expect and it certainly does not
> seem to exhibit behavior similar to a TreeView on a TreeModel.

It is supposed to do so (and indeed the "Moen Tree Browser" in the
Additional Tools folder demonstrates that it can be plugged in as a
replacement for the standard TreeView, since the Moen browser is just a new
view onto the standard presenter).

>...I've looked
> at DSDN and seen some posts but those don't seem to help, so after a week
of
I think that may be something to do with the behaviour of the TreeView
control itself.

The MoenTreeView is designed to be used within a ScrollingDecorator. As I
explained in a posting of 5 Nov. 1999:
"...the MoenTree is unlike a normal TreeView in that it doesn't implement
its own scrolling. In fact its instances have to be embedded in
ScrollingDecorators. There might be a case where one doesn't want the
scrolling support, but it seems unlikely since one doesn't know in advance
how large the tree will turn out to be (it is up to the MoenTree to
calculate its extent)."

>...So now try this:
> tp := TreePresenter create: 'Moen tree' in: (CompositePresenter show:
> 'Scrolling container') on: TreeModel new.
> tp model add: View asChildOf: nil
> tp model add: Object asChildOf: nil
> tp model move: View asChildOf: Object
> Nothing appears in the view after either of the first two adds and a
> walkback appears after the move, saying that View isn't found.
Something's
> starting to make a little sense now, i.e., even though the model triggers
an
> event, the view is not adding the nodes.

There appears to be a bug in the MoenTreeView in respect of the addition of
new root nodes to its TreeModel (it doesn't display them). Try this:

tp := TreePresenter create: 'Moen tree' in: (CompositePresenter show:
'Scrolling container') on: (TreeModel withRoots: (Array with: Object)).
tp model add: View asChildOf: Object.
Any attempt to add a new root node fails.

> ...
> I wouldn't be surprised if I'm missing something here (standard brain
> disclaimer) but having to send #refreshContents seems rather heavyhanded
and
> duplicative -- the model triggers add, move, etc events which the view
> should pick up on.  It also indicates the MoenTreeView is not as pluggable
> as one would think.  I've kept this example brief, but I think it's even
> worse when one gets into adding, moving, and removing more than one class
at
> a time -- I know in some cases even #refreshContents didn't do the job and
I
> had to send "tp model: tp model" (which really digs into my aesthetic
> nerve)....

It is indeed supposed to update automatically and be a plug replacement for
the TreeView (with the exception that it must be parented in a
ScrollingDecorator), but it just has problems with root nodes. I found 3
separate issues related to adding, moving, or deleting root nodes,
preliminary fixes for which are in the attached patch.

>...I was hoping to implement, among other things, a graphical browser
> that shows a subset of classes.

Presumably you have tried the Moen Tree (Class) Browser in Local Hierarchy
mode?

Regards

Blair

-----------------------------------------------------------
!MoenTreeView methodsFor!

linkChild: node toParent: pNode
 "Private - The <MoenTreeNode>, node, has been added (or perhaps moved) into
the
 Moen tree beneath the node of the element, aParentObject (which can be nil
if the
 node is becoming a root). We must adjust the contours to accomodate the new
node.
 Note that the new node is already linked into the tree."

 | sNode |
 pNode isNil
  ifTrue: [roots addLast: node]
  ifFalse: [
   self unzip: pNode.
   sNode := pNode child.
   pNode child: node].
 node parent: pNode.
 node sibling: sNode.
 self treeLayout: node.
 pNode notNil
  ifTrue: [self zip: pNode]
  ifFalse: [self zip: node]!

onItem: object removedFromParent: parent
 "Event received when the <Object>, object, has been removed from the
receiver's
 model from the parent <Object>, parent. The parent may be nil if a root
item is being
 removed. Note that the object (and indeed parent) may appear more than once
in the
 tree if the underlying model is not really a true tree but a directed
graph."

 | nodes candiateNodes |
 nodes := (self nodesForObject: object).
 nodes := parent isNil
  ifTrue: [nodes select: [:node | node parent isNil]]
  ifFalse: [ | discriminator |
   discriminator := self model searchPolicy.
   nodes select: [:node | | p | (p := node parent) notNil
        and: [discriminator compare: parent with: p object]]].
 nodes do: [:node |   | parentNode |
  parentNode := node parent.
  selection == node ifTrue: [
   "Try and move selection to the parent (or nil)"
   selection := nil.
   self basicSelection: parentNode].
  self removeTree: node].
 self generateAbsolutes; invalidateLayout; invalidate!

removeTree: node
 "Private - Remove the sub-tree rooted at the <MoenTreeNode>, node, below
the <MoenTreeNode>
 parent node, pNode."

 | sNode dNode cNode pNode |
 pNode := node parent.
 pNode isNil ifTrue: [ "if anObject is a root just update roots collection
and that's it"
  roots remove: node.
  ^self].
 self unzip: pNode.
 cNode := pNode child.
 sNode := node sibling.
 cNode == node
  ifTrue: [pNode child: sNode]
  ifFalse: [
   dNode := cNode sibling.
   [dNode ~~ node] whileTrue: [cNode := dNode. dNode := dNode sibling].
   cNode sibling: sNode].
 self zip: pNode.! !

!MoenTreeView categoriesFor: #removeTree:!private!removing! !
!MoenTreeView categoriesFor: #linkChild:toParent:!adding!private! !
!MoenTreeView categoriesFor: #onItem:removedFromParent:!event
handling!public! !

Blair,

Thanks so much for your response.  I implemented the changes and it works
now (though I still have some other redesign work to do).  Since a picture
is worth a thousand words, here's a pic of the browser showing just a few
classes, and under each class, some of the branches.

http://www.sirius.com/~lsumberg/Dolphin/LasClassBrowserShell1.jpg

I still need to do some usability testing for myself to see if this is truly
worth it as a regular-use type of browser, but in the meantime it does seem
helpful if I want to look at different subsets of the class hierarchy at
once.  I realize the local hierarchy mode toggle and the history list in the
regular browsers help one to jump around easily -- and I use them
extensively -- but I thought it might be nice to have the classes I'm
working on, or taking from (*smiles*), right up there, though I run the risk
of having a big monolithic browser.

-- Louis

Ian

Thanks for your thoughts on this ... so so humble you are, i.e., your "vague
thought".  You're right on the money (whatever the currency may be) and I
will probably make changes for the performance reasons you've pointed out.
Initially, I wanted to use what's already there and so far there isn't a big
problem because I'm usually adding/removing a small number of nodes at a
time (running on a P3/450MHz).  But if I add the whole Object hierarchy, for
instance, it does take a long time (a minute, more or less).

I still get confuzzled with events, views and presenters, especially where
sometimes the event goes to the view and sometimes to the presenter.  In
this case, since MoenTreeView sets up observers directly on the TreeModel,
it does the update directly.  I assume one approach would be to remove those
observers when the shell is created, and as you point out, have the
application do the update.  I've already subclassed ClassHierarchyPresenter
so that might be a good place to do it.  Then again, it'd be cleaner I think
if the model allowed for bulk adds and triggered a tree change at that
point.  I'll look closer at that, but right now I believe there's only an
add child method available.

-- Louis

Louis,

> In this case, since MoenTreeView sets up observers directly on the
> TreeModel, it does the update directly.  I assume one approach
> would be to remove those observers when the shell is created, and
> as you point out, have the application do the update.  I've already
> subclassed ClassHierarchyPresenter so that might be a good place
> to do it.  Then again, it'd be cleaner I think if the model allowed for
> bulk adds and triggered a tree change at that point.  I'll look closer
> at that, but right now I believe there's only an add child method
> available.

I think I'd be more inclined to treat it like a buffered graphic (which in
some ways I suppose it is!) and keep a separate working version of the
TreeModel which was not attached to any view. Make all updates to this model
and when you have finished replace the TreeView's model with a copy [1] of
the working model. The only awkward bit might be recognising any input into
the TreeView (drag/drop for example) and replicating that in your model -
but it shouldn't be too difficult.  You wouldn't even have to refresh the
view yourself as changing the TreeView's model should will do it for you.

[1] You'll either need to do a #deepCopy or build a new copy up yourself
using #add, before passing the TreeModel copy to the TreeView of course.
#deepCopy would be easiest but might have unwanted effects on the objects
contained within the tree and can sometimes cause stack recursion problems
if the tree is too large (as I just discovered <g>)

I tried a quick experiment. Using a copied "working" model, below, takes
just over 1 second on my box

working := TreeModel new.
0 to: 9 do: [:index | working add: index asChildOf: nil].

tp := TreePresenter
    create: 'Moen tree'
    in: (CompositePresenter show: 'Scrolling container')
    on: working deepCopy.

0 to: 1000 do: [:index | working add: index asChildOf: index // 10].
tp model: working deepCopy.

working move: 5 asChildOf: 90.
tp model: working deepCopy

whereas updating the existing model takes about 12 seconds (with Blair's
patches installed)

working := TreeModel new.
0 to: 9 do: [:index | working add: index asChildOf: nil].

tp := TreePresenter
    create: 'Moen tree'
    in: (CompositePresenter show: 'Scrolling container')
    on: working.

0 to: 1000 do: [:index | working add: index asChildOf: index // 10].

working move: 5 asChildOf: 90

Ian

Ian, Louis

"Ian Bartholomew" <[hidden email]> wrote in message
news:9t1z6.80641$[hidden email]...
>
> I think I'd be more inclined to treat it like a buffered graphic (which in
> some ways I suppose it is!) and keep a separate working version of the
> TreeModel which was not attached to any view. Make all updates to this
model
> and when you have finished replace the TreeView's model with a copy [1] of
> the working model....

Why not just suppress event generation for the duration of the "batch" of
changes and then send out a general #treeChanged: event at the end, e.g.

working := TreeModel new.
0 to: 9 do: [:index | working add: index asChildOf: nil].
tp := TreePresenter
    create: 'Moen tree'
    in: (CompositePresenter show: 'Scrolling container')
    on: working.
[working noEventsDo: [
    0 to: 1000 do: [:index | working add: index asChildOf: index // 10].
    working move: 5 asChildOf: 90]] ensure: [working refreshTree: nil]
This takes 147mS on my machine, which from comparing figures would appear to
be somewhat slower than Ian's (its a dual PII 400, rather old hat now).

Regards

Blair

Blair,

> Why not just suppress event generation for the duration of the "batch" of
> changes and then send out a general #treeChanged: event at the end, e.g.

There's an easy answer to that (from me at least) involving the word
"ignorance" <g>

There used to be columns in magazines (Readers Digest rings a bell?) that
introduced unusual words from a dictionary and gave examples on when and
where you should use them. Perhaps we should start an "Unusual Dolphin
method of the month" page on the WIKI....

> This takes 147mS on my machine, which from comparing figures would appear
> to be somewhat slower than Ian's (its a dual PII 400, rather old hat now).

I've got a 750 P3 and it takes 71 mS, which sounds about right in relation
to your figure

Ian

Ian

You wrote in message news:Ds5z6.72492$[hidden email]...
>
> > Why not just suppress event generation for the duration of the "batch"
of
> > changes and then send out a general #treeChanged: event at the end, e.g.
>
> There's an easy answer to that (from me at least) involving the word
> "ignorance" <g>

Actually I was ignorant of the #treeChanged: event myself (though I had the
advantage of knowing about the #noEventsDo:), so I was going to just ask the
view to #refreshContents, but when I looked at the senders of
#refreshContents I found the event. "Great," I thought as I put my
code-shopping hat away.

>
> There used to be columns in magazines (Readers Digest rings a bell?) that
> introduced unusual words from a dictionary and gave examples on when and
> where you should use them. Perhaps we should start an "Unusual Dolphin
> method of the month" page on the WIKI....

Why not. Any entry for #noEventsDo: ought to mention (as I should have done
before) that, like many optimisations, it is potential source of errors
since one has to be sure that the observers eventually get notified of the
changes. In this case #treeChanged: fits the bill rather nicely.

>
> > This takes 147mS on my machine, which from comparing figures would
appear
> > to be somewhat slower than Ian's (its a dual PII 400, rather old hat
now).
>
> I've got a 750 P3 and it takes 71 mS, which sounds about right in relation
> to your figure

Hmmm. Time for an upgrade methinks. I wonder how much a pair of 1.5Ghz
Pentium 4s would be? Oh dear, £582+VAT each.

Regards

Blair

#noEventsDo: ...???  Wow, that's a kewl one -- learn something new every
day.  Thanks, Blair!

I implemented the bulk add and remove (MoenTreeView nodes) by removing the
observer to the model (as I indicated in another post), having the presenter
send #refreshContents, and it sure zips through them in a hurry.  I was
going to change to using #noEventsDo:, but ran into a problem which I'm sure
would be the same as would happen with using #noEventsDo:.  The problem has
to do with the side effects of using #refreshContents (or #treeChanged:).
If I'm editing a method in a class, then decide to show (add) the subclasses
of that class, #refreshContents resets the selected class, which in turn
resets the selected method.  I could, of course, save and restore the
selections (class, category, method) but it starts sounding ugly, and
besides, for a treeview I think it also collapses all nodes.  I'll probably
put in a test where if the total number of existing nodes plus those to be
added is greater than some threshold number, it goes the bulk route, else it
adds them one at a time.

I do have a question on the MoenTreeView -- is there any way to adjust the
vertical spacing between root nodes?  Right now if you have root nodes with
no children beneath them, it's obviously spacing the root nodes too far
apart.  I tried noseying around but didn't get anywhere useful, just crashed
a few times.  It would be particularly useful for my version of the Moening
class browser, where real estate in the classes view is rather limited.

On another subject, I think there's a little buglet in
Package>>#beNotUsingPAX.  I tried to switch a package from Pax back to Pac,
it prompted me do I really want to delete x number of files, I said yes, and
then it couldn't continue because one of the files wasn't there. So I edited
the last line in #beNotUsingPAX to test if the files exist, like so:

    (self allFileOutNames select: [:file | File exists: file]) do: [:each |
File delete: each]].

Thanks again.

-- Louis