Why can I only have 1 presenter per view? It seem very non MVP

I am creating a little toy timer application like this:

Duration: xx  [||||| progress bar]

<Start> < Pause>


I have a NumberPresenter hooked up to the duration so I can edit it, and
its works well. I also have the progress bar hooked up as a Presenter to
the elapsedTime of my model.

However, I also want to hook up the duration to the #range: of the progress
bar (e.g. if I type 120, my bar should rescale).

This sounds really simple - i created a new RangePresenter that takes the
duration from my model and creates an appropriate Interval for the #range:
of the progress bar.

Great I thought - I'm starting to get the hang of this - however when I wire
it up in my Shell application:

super createComponents. "(AutoGenerated on 12:03:45, 09 February 2006)"
progressEstimatePresenter := self add: Presenter new name: 'progressEstimate'.
estimateIntervalPresenter := self add: ProgressRangePresenter new name: 'progressEstimate'.

I get a walkback - "duplicate name: progressEstimate"

And looking in the code - sure enough it checks for this. But I don't understand
why?

 From my basic understanding of MVP what I have done seems reasonable - why
can't my model update different aspects of the UI through crafted presenters?

It seems that this restriction means that I would have to subclass the Progress
widget, and get it to understand a model that has not just a value, but also
a max range (you seem to be only able to have one model per widget from what
I see). I then need to write a new presenter for that model.

This seems to greatly reduce widget reuse - and magnify the number of widgets
and presenters? E.g. the same would occur for a text entry widget where I
also want to be able to set the background colour from another widget.

Of course I can do all of this by hooking up extra events - and then writing
code to handle a trigger and manipulate the widgets but that doesn't seem
like the model MVP approach I was being sold.

Have I missed something important? It doesn't look too hard to make the change
that maintains a dictionary of lists of presenters... although it would probably
break some UI that expect a 1 to 1 mapping.

Tim

*IF* I am following you, there really isn't a problem: just create a
second presenter attached to a view of a different name
(progressEstimate2???) and share the model between them.  Does that
sound like it will help?

Have a good one,

Bill

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

macta wrote:

> Have I missed something important? It doesn't look too hard to make the
> change
> that maintains a dictionary of lists of presenters... although it would
> probably
> break some UI that expect a 1 to 1 mapping.

It would break just about everything -- the entire architecture is set up so
each View has exactly one Presenter, and each Presenter has exactly one View.
Models, and only Models, can be shared, between MVP triads.

I think that the problem you are having here is not that MVP doesn't allow
multiple Presenters per View, but that subsidiary aspects of a View are not
directly modelled using the MVP style.  I.e. the #isReadOnly aspect of a text
view is not controlled by a Boolean-valued model, the #range aspect of a
ProgressBar is not a reflection of a Number-valued model, and so on.

I found that surprising and disappointing too, when I started out with Dolphin
MVP.  Now I no longer think about it (except that, as it happens, some of my
stuff is developing such complicated configuration that it makes sense to
factor it out into separate "display models").   The main problems is that all
those little models would quickly become unmanageable.

But Dolphin does have the ability to create virtual models from aspects of
objects.  The specific example you've chosen is awkward since there's no
built-in RangePresenter; rather than complicate the example, I've just added
#rangeUpper and #rangeUpper: to ProgressBar with the obvious definitions.  With
those added you can say:

    pb := NumberPresenter show: 'Progress bar'.
    pb value: 50.    "just so there's something to see"
    rangeModel := pb view aspectValue: #rangeUpper.

now rangeModel is an adaptor for the #rangeUpper aspect, so we can say:

    rangeModel value: 200.

and the view will update accordingly.  We can attach a Number presenter to the
same model

    np := NumberPresenter showOn: rangeModel.

and now the two will keep step as you edit the number.

That sort of technique can be used in lots of cases.  The one where it doesn't
work is when the underlying model is given beforehand and you want to "tie"
various aspects of your views to that model's value.  As it happens, Dolphin
doesn't come with the necessary adaptors for this case, but they are easy to
add.  I'll append a small class which offers one way of doing it.  With that
loaded you can say:

    tp := TextPresenter show.
    v := tp view.
    v hasClientEdge: false.

    cluster := (ValueCluster new)
                        addAspect: #isReadOnly of: v;
                        addAspect: #isStatic of: v;
                        addAspect: #hasStaticEdge of: v;
                        yourself.

and "cluster" is now a single Model which ties three aspects of the text view
together.

    cluster value. "--> false"

Evaluating either of the following will change all three slave aspects
together:

    cluster value: true.
    cluster value: false.

For a more complicated example which ties several aspects (inlcuding the text
itself) to a "prior" Boolean-valued model ("readonly" in this case):

    "our 'pior' model"
    readonly := false asValue.

    "show a text presenter on a derived model"
    tm := ValueConverter
                    subject: readonly
                    typeConverter: BooleanToText new.
    tp := TextPresenter showOn: tm.
    v := tp view.

    "create a Model equivalent to
        v hasCientEdge not
    "
    not := PluggableTypeConverter leftToRight: [:b | b not] rightToLeft: [:b |
b not].
    notClient := ValueConverter
                            subject: (v aspectValue: #hasClientEdge)
                            typeConverter: not.

    "tie all those together with 'readonly' as the master"
    cluster := (ValueCluster new)
                        addSlave: readonly;
                        addAspect: #isReadOnly of: v;
                        addAspect: #isStatic of: v;
                        addAspect: #hasStaticEdge of: v;
                        addSlave: notClient;
                        yourself.

And now changing the value of "readonly" will propogate though to both the main
Model of the MVP triad, but also to several of the View's aspects:

    readonly value: true.
    readonly value: false.

I hope that makes some sort of sense...

    -- chris

========== filein =============
"Filed out from Dolphin Smalltalk XP"!

ValueHolder subclass: #ValueCluster
 instanceVariableNames: 'slaves isSettingSlaves'
 classVariableNames: ''
 poolDictionaries: ''
 classInstanceVariableNames: ''!
ValueCluster guid: (GUID fromString: '{3B80026E-E854-489D-8AB5-51DCED2C5EDD}')!
ValueCluster comment: 'Copyright © Chris Uppal,2006.

One of these is a ValueHolder which additionally holds a collection of other
ValueModels (typically ValueAdaptors) and constrains them all to have the same
value.'!
!ValueCluster categoriesForClass!Unclassified! !
!ValueCluster methodsFor!

addAspect: anAspectSymbol of: anObject
 "convenience method, add a ValueAdaptor reading
 the given aspect of the given object to our list of
 slaves.
 Answers the new slave"

 ^ self addSlave: (anObject aspectValue: anAspectSymbol).!

addAspect: anAspectSymbol trigger: anEventSymbol of: anObject
 "convenience method, add a ValueAdaptor reading
 the given aspect of the given object to our list of
 slaves.  We will expect anEventSymbol to be triggered
 off the object when the aspect is changed.
 Answers the new slave"

 ^ (self addAspect: anAspectSymbol of: anObject)
  aspectTriggers: anEventSymbol;
  yourself.!

addSlave: aValueModel
 "add aValueModel to our list of slaves.
 Normally, when a slave is added, it will be forced to use our
 current value and comparison policy.  As a hacky, but convenient,
 feature, if our current value is nil (the default) and we have no prior slaves
 then we 'borrow' those value from the new slave.
 Answers the slave"

 (slaves isEmpty and: [self value isNil])
  ifTrue: [self
    comparisonPolicy: aValueModel comparisonPolicy;
    value: aValueModel value]
  ifFalse: [aValueModel
    comparisonPolicy: self comparisonPolicy;
    value: self value].

 aValueModel
  when: #valueChanged
  send: #onSlaveChanged:
  to: self
  with: aValueModel.

 ^ slaves add: aValueModel.!

comparisonPolicy: aComparisonPolicy
 "overriden to force the comparison policy through to our sub-models"

 super comparisonPolicy: aComparisonPolicy.
 slaves do: [:each | each comparisonPolicy: aComparisonPolicy].!

initialize
 "private -- establish a coherent initial state"

 slaves := OrderedCollection new.
 isSettingSlaves := false.
 super initialize.
!

onSlaveChanged: aValueHolder
 "private -- one of our slaves has changed value"

 isSettingSlaves ifFalse: [self value: aValueHolder value].

 !

setValue: anObject
 "private -- overriden to force the new value through to our slave models"

 | wasSettingSlaves |

 super setValue: anObject.

 "NB: we use #value: not #setValue: since our slaves will have their own
observers,
 however we ourself are also an observer so we have to allow for that"
 wasSettingSlaves := isSettingSlaves.
 isSettingSlaves := true.
 [slaves do: [:each | each value: anObject]]
  ensure: [isSettingSlaves := wasSettingSlaves].

! !
!ValueCluster categoriesFor: #addAspect:of:!adding!public! !
!ValueCluster categoriesFor: #addAspect:trigger:of:!adding!public! !
!ValueCluster categoriesFor: #addSlave:!adding!public! !
!ValueCluster categoriesFor: #comparisonPolicy:!accessing!public! !
!ValueCluster categoriesFor: #initialize!initializing!private! !
!ValueCluster categoriesFor: #onSlaveChanged:!event handling!private! !
!ValueCluster categoriesFor: #setValue:!accessing!private! !
===========================

Hi Chris,

I can see what you are getting at - its sort of the "other way around to
what I was thinking". I really appreciate the time you take to answer people's
questions with so much useful detail.

If I've understood you correctly - I can almost get my example to work.

I can't just hook up my duration NumberPresenter to the #rangeUpper of my
ProgressBar as my GameTimerModel also needs to know about this value (I want
to do something extra when the time expires).

So then I looked at your ValueCluster - and this looked like what I needed,
but I hit two problems:

I put code in the #model: of my Shell class as follows:

cluster := (ValueCluster new)
                addAspect: #rangeUpper of: progressEstimatePresenter view;
                addAspect: #estimateTimerDuration of: aGameTimerModel;
                yourself.
       
        estimateTimePresenter model: (cluster).

Problem #1: where I was hooking up my presenters in the #model: method of
my view (copied from the PersonalAccount example), the #addSlave: method
was doing "aValueModel value" which caused a walkback as the progress bar
was still a deafObject. So I commented that bit out (I can come up with another
mechanism for specifying a default value - and your comment indicates its
a hack anyway.

Problem #2: My gameTimerModel updates, but my progressBar doesn't - and I
wonder if its becuase the Cluster has the DeafObject and not the real object
in its list of slaves?

So should I be doing something different - or is this a case you didn't think
of?

Tim

So I

macta wrote:

> Problem #1: where I was hooking up my presenters in the #model: method of
> my view (copied from the PersonalAccount example), the #addSlave: method
> was doing "aValueModel value" which caused a walkback as the progress bar
> was still a deafObject. So I commented that bit out (I can come up with
> another mechanism for specifying a default value - and your comment
> indicates its a hack anyway.

There's a generic problem with the MVP framework that #model: can be called
before the view is set up.  Often that doesn't cause problems, but in some
cases (around 1/3 in my experience) it does.  I don't know of any general
solution, so I just code around it when problems crop up (usually cursing aloud
as I do so, and occasionally cursing in comments too).  Simplest way typically
is to guard the view-sensitive parts of #model: with a

    (self view isKindOf: DeafObject)

test, and to add an override of #onViewAvailable or #onViewOpened which
executes

    self model: self model.

(don't forget the super-send! ;-)  There are many other ways of expressing the
same general idea.  I think you'll find that the simple "just code around it"
approach works better than the fancy footwork that you've described in other
posts.

(As an aside, OA once seemed quite enthusiastic about the idea of using a
special MVP-specific subclass of DeafObject for the setup stages of an MVP
triad.  It could provide sensible implementations of more of the methods which
Views support (e.g. #isOpen to answer false would make life easier).
Unfortunately the idea seems to have been forgotten.)

A second point is that the way ValueCluster works is that it initialises its
state from the first ValueModel it is given, so if you have a /real/ model
somewhere (the "prior" of my earlier post) then it makes sense to feed that to
the ValueCluster first.

A last point is that your new class-side #default: method isn't necessary -- it
essentially duplicates the #with: method inherited from ValueHolder ;-)

(BTW: thanks for this feedback -- it's given me a number of ideas for improving
the API of ValueCluster.  For instance, I'll probably split #addSlave: into
#addMaster: and #addSlave: which differ only in whether the cluster adopts the
state of the new Model, or forces the new Model into its own state.  Also I may
add a class-side #withMaster: factory method.)


> Problem #2: My gameTimerModel updates, but my progressBar doesn't - and I
> wonder if its becuase the Cluster has the DeafObject and not the real
> object in its list of slaves?

That would certainly have that effect.

Another potential problem to be aware of (which should really be in the
ValueCluster class comment -- except that I haven't written it yet ;-) is that
you have to pay attention to which Models supply change notification.  My
Models normally do (that's just how I like to write 'em), but most don't.  For
instance if you have some object, anObject, which has an aspect, #something,
then it /may/ trigger #somethingChanged notifications itself, but probably it
will not do so.  If you add that aspect to a cluster with code like:

    cluster addAspect: #something of: anObject.

then the cluster will attempt to hook itself up to the new (internal)
ValueModel's change notifications, and that will work fine if you change the
aspect's value only by via the cluster itself (including its other slaves), or
by sending #value: to the ValueModel answered by #addAspect:of:.  If there is
other code in your system which changes the value directly by sending
#something: to anObject, then that will be "invisible" to the cluster.

There's nothing that can be done about that (short of changing Smalltalk
semantics completely), and it's not specific to ValueCluster (most of the
ValueAdaptors have similar problems), but it's something to be aware of.

    -- chris