On Thu, Sep 27, 2012 at 10:49 AM, Sven Meier <s...@meiers.net> wrote:
> We're discussing the case where a component distributes its model to its
> children or behaviors.
> For the component developer it's easier to assume that the model can't be
> changed without its consensus (i.e. by offering a generic #setModel()).
>
> Of course this can be handled perfectly by a coding guideline. I always tell
> people to *never* change a component's model.
> I cannot count times where I'm called to a developer's IDE with him having
> absolutely no clue why something entered here doesn't display over there: In
> many cases this is caused by setting models.
>
> But a protected #setDefaultModelObject() would make this explicit in the
> API.
ok. lets start with a bit of history to have more context.
setDefaultModel() only exists because of type-erasure. before wicket
supported generics all components had a public setModel() method. so,
one might say that having a public setModel() is "the wicket way"
because it was there since 1.0. just to establish the baseline.
lets take a concrete example of FormComponent. right now it has a
public setModel() method, but by your thinking we would have to make
both setDefaultModel and setModel methods protected, because we do not
know that all FormComponents support changing the model. after all, a
common subclass of FormComponent is FormComponentPanel which pretty
much always distributes its model. so, we leave it to subclasses of
FormComponent and FormComponentPanel to decide whether or not to
override setModel() to make it public.
a TextField would make its setModel() public - because it properly
handles the usecase, correct? so it is still possible for your
developers to call setModel() on a textfield and rewire it so it no
longer links with a model of another component correctly.
so we are now back to square one with the addition that a lot of
components have to override setModel() just to change its visibility
from protected to public - introducing a lot of noise.
im all for making the code better, but i do not think that this change
does. in the end, the developer has to know what the method does if
they chose to call it.
-igor
>
> Sven
>
>
>
> On 09/27/2012 07:09 PM, Igor Vaynberg wrote:
>>
>> i thought the issue we were discussing here is the way the models are
>> linked...which is not solved by making setdefaultmodelobject
>> non-public.
>>
>> -igor
>>
>> On Thu, Sep 27, 2012 at 10:04 AM, Sven Meier <s...@meiers.net> wrote:
>>>
>>> If you extend GenericPanel you inherit setModel() and getModel(), that's
>>> the
>>> whole purpose of this base class.
>>> You want these two methods, otherwise you wouldn't extend it - there's
>>> nothing to fix.
>>>
>>> Component#setDefaultModel() is dangerous because it allows others to
>>> tinker
>>> with your component innards.
>>>
>>> I still think limiting access to #setDefaultModel() is a good idea, but
>>> this
>>> is no crucial issue anyway.
>>>
>>> Sven
>>>
>>>
>>>
>>> On 09/27/2012 06:16 PM, Michael Mosmann wrote:
>>>>
>>>> Am 27.09.2012 17:51, schrieb Igor Vaynberg:
>>>>
>>>> good point..
>>>> -1 from me.. thought it was a good idea, but wasn’t
>>>>
>>>> Michael
>>>>
>>>>> so what happens if panel A extends GenericPanel which has setModel?
>>>>> you havent fixed anything.
>>>>>
>>>>> -igor
>>>>>
>>>>> On Thu, Sep 27, 2012 at 8:50 AM, Michael Mosmann <mich...@mosmann.de>
>>>>> wrote:
>>>>>>
>>>>>> Am 27.09.2012 17:32, schrieb Igor Vaynberg:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> .. i would leave setModel as it is, only make this change for
>>>>>> Component.setDefaultModel().
>>>>>>
>>>>>> Michael
>>>>>>
>>>>>>> -1 on changing setDefaultModel().
>>>>>>>
>>>>>>> 1) if B panel's model is truly dependent on A's then that dependency
>>>>>>> should be expressed:
>>>>>>>
>>>>>>> add(new BPanel("b", new PropertyModel(this, "defaultModel"));
>>>>>>>
>>>>>>> or do not use the default model slot of B to store the model. that
>>>>>>> way
>>>>>>> setDefaultModel() calls on B will be a noop and you can choose not to
>>>>>>> provide a setter.
>>>>>>>
>>>>>>> 2) you are only "solving" this for a subset of usecases where the
>>>>>>> container (A) is not generic. are we also going to make setModel(T)
>>>>>>> protected? that would require the model assignment be done through
>>>>>>> the
>>>>>>> constructor only and would eliminate any possibility of writing
>>>>>>> builder-style code. consider a simple example:
>>>>>>>
>>>>>>> new DropDownChoice("foo").setModel(bar).setChoices(baz)...
>>>>>>>
>>>>>>> this kind of code should be possible whether written directly by the
>>>>>>> developer in the page or produced by some builder.
>>>>>>>
>>>>>>> -igor
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Sep 27, 2012 at 5:19 AM, Sven Meier <s...@meiers.net> wrote:
>>>>>>>>
>>>>>>>> Even a simpler example might fail (no PropertyModel involved):
>>>>>>>>
>>>>>>>>
>>>>>>>> class APanel extends Panel {
>>>>>>>> APanel(String id, IModel<Some> model) {
>>>>>>>> super(id,model);
>>>>>>>>
>>>>>>>> add(new BPanel("b", model);
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> A client using APanel might later change the model, leaving BPanel
>>>>>>>> working
>>>>>>>> on the old model:
>>>>>>>> aPanel.setDefaultModel(otherModel);
>>>>>>>>
>>>>>>>> You could argue that APanel should be made failsafe when passing the
>>>>>>>> model:
>>>>>>>>
>>>>>>>> add(new BPanel("b", new PropertyModel(this,
>>>>>>>> "defaultModel")));
>>>>>>>>
>>>>>>>> But it would be much easier if APanel could assume that its model
>>>>>>>> isn't
>>>>>>>> changed unattendedly.
>>>>>>>>
>>>>>>>> IMHO changing a component's model isn't the "wicket way" so I'd
>>>>>>>> suggest
>>>>>>>> changing the visibility of Component#setDefaultModel() to protected.
>>>>>>>>
>>>>>>>> Sven
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 09/27/2012 10:47 AM, Martin Grigorov wrote:
>>>>>>>>>
>>>>>>>>> I see. This is an advanced way to create a static model :-)
>>>>>>>>> But again I find PropertyModel as the real problem here.
>>>>>>>>>
>>>>>>>>> I'll let others give their opinions too.
>>>>>>>>>
>>>>>>>>> On Thu, Sep 27, 2012 at 11:39 AM, Michael Mosmann
>>>>>>>>> <mich...@mosmann.de>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Am 27.09.2012 09:51, schrieb Martin Grigorov:
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> a dont care about the type issue here.. Maybe i can explain it
>>>>>>>>>> again
>>>>>>>>>> in
>>>>>>>>>> an
>>>>>>>>>> other way:
>>>>>>>>>>
>>>>>>>>>> APanel uses model instance A and the label uses a property model
>>>>>>>>>> instance
>>>>>>>>>> P
>>>>>>>>>> which uses a reference to model instance A.
>>>>>>>>>>
>>>>>>>>>> After calling APanel.setDefaultModel(B) APanel uses model instance
>>>>>>>>>> B,but
>>>>>>>>>> label uses model instance P which uses model instance A as before.
>>>>>>>>>> So
>>>>>>>>>> the
>>>>>>>>>> label does not see any changes, because no one tells the model
>>>>>>>>>> instance
>>>>>>>>>> P,
>>>>>>>>>> that it should use B instead of A. I think, there are rare cases
>>>>>>>>>> for
>>>>>>>>>> such
>>>>>>>>>> a
>>>>>>>>>> usage.
>>>>>>>>>>
>>>>>>>>>> thanks
>>>>>>>>>> Michael
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> In this particular code I think the "problem" is PropertyModel,
>>>>>>>>>>> since
>>>>>>>>>>> it brings the type unsafety.
>>>>>>>>>>>
>>>>>>>>>>> Another solution is to make Component<T>, this way we can remove
>>>>>>>>>>> #setDefaultModel() and have #setModel(IModel<T>) only and such
>>>>>>>>>>> problems will go away.
>>>>>>>>>>> But as discussed in early Wicket 1.4 days this will lead to more
>>>>>>>>>>> typing. With Java 7 diamonds it is half the typing though.
>>>>>>>>>>>
>>>>>>>>>>> For now you can use GenericPanel, GenericPage and all
>>>>>>>>>>> FormComponent.
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Sep 27, 2012 at 10:41 AM, Michael Mosmann
>>>>>>>>>>> <mich...@mosmann.de>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Am 27.09.2012 09:01, schrieb Martin Grigorov:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I think, there is a little difference in using setDefaultModel
>>>>>>>>>>>> and
>>>>>>>>>>>> setDefaultModelObject .. the first one sets a new model
>>>>>>>>>>>> instance,
>>>>>>>>>>>> the
>>>>>>>>>>>> second
>>>>>>>>>>>> only change the value in the existing model. Some pseudo-code:
>>>>>>>>>>>>
>>>>>>>>>>>> class APanel extends Panel {
>>>>>>>>>>>> APanel(String id,IModel<Some> model) {
>>>>>>>>>>>> super(id,model);
>>>>>>>>>>>>
>>>>>>>>>>>> add(new Label("name",new
>>>>>>>>>>>> PropertyModel(getDefaultModel(),"name"));
>>>>>>>>>>>> }
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> If you replace the value in model, everything is fine and works
>>>>>>>>>>>> as
>>>>>>>>>>>> expected.
>>>>>>>>>>>> If you call setDefaultModel you might think, that everything is
>>>>>>>>>>>> fine,
>>>>>>>>>>>> but
>>>>>>>>>>>> its not. A child component does not use
>>>>>>>>>>>> getParent().getDefaultModel()
>>>>>>>>>>>> to
>>>>>>>>>>>> get
>>>>>>>>>>>> these changes. I saw a lot of code like this, which leads to
>>>>>>>>>>>> trouble,
>>>>>>>>>>>> if
>>>>>>>>>>>> you
>>>>>>>>>>>> change the model and not the value.
>>>>>>>>>>>>
>>>>>>>>>>>> If there is no benefit in using setDefaultModel over
>>>>>>>>>>>> setDefaultModelObject i
>>>>>>>>>>>> would like to remove this method. This could prevent many "you
>>>>>>>>>>>> might
>>>>>>>>>>>> not
>>>>>>>>>>>> got
>>>>>>>>>>>> the full picture how to use wicket the right way" errors.
>>>>>>>>>>>>
>>>>>>>>>>>> Michael
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Most of the time it is recommended to use a dynamic model, so
>>>>>>>>>>>>> there
>>>>>>>>>>>>> is
>>>>>>>>>>>>> no reason to replace the component's model.
>>>>>>>>>>>>> Component#setDefaultModel() gives you semi-dynamic nature - you
>>>>>>>>>>>>> can
>>>>>>>>>>>>> replace the model completely with a new one. Same with
>>>>>>>>>>>>> #setDefaultModelObject().
>>>>>>>>>>>>>
>>>>>>>>>>>>> What is the problem you face with it ?
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Sep 27, 2012 at 9:57 AM, Michael Mosmann
>>>>>>>>>>>>> <mich...@mosmann.de>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> is there any usefull application of
>>>>>>>>>>>>>> Component.setDefaultModel(...)?
>>>>>>>>>>>>>> IMHO
>>>>>>>>>>>>>> this Method is the cause for much trouble without any benefit.
>>>>>>>>>>>>>> But
>>>>>>>>>>>>>> maybe
>>>>>>>>>>>>>> i
>>>>>>>>>>>>>> did not understand when someone should replace a component
>>>>>>>>>>>>>> model...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> thanks
>>>>>>>>>>>>>> Michael
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>
