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
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>
>
