for the component itself we already do that:
    // Component is removed
        component.setParent(null);

But ofcourse this could not be the case that the component itself is not removed but a parent of it and he is reused instead of its parent.
But this is pretty far fetched don't think this will happen a lot.

I just checked in the code where i just remove the component first if he still has a parent.

johan


On 12/22/05, Juergen Donnerstag <[EMAIL PROTECTED]> wrote:
You mentioned that throwing an exception won't work because if
Components are moved around (for whatever reason), the
markupContainer.parent could already be set. IMO Wicket should do
markupContainer.parent = null whenever we remove a Component from the
hierarchy and that happens in markupContainer.remove(Component) and
replace(Component). If we make sure parent == null, than we are able
to throw an exception and IMO that is the better option.

Juergen


On 12/22/05, Johan Compagner <[EMAIL PROTECTED]> wrote:
> remove the parent reference? That already happens because we set an other
> parent ofcourse.
> But just do it as swing does this is swing/awt code:
>
>   /* Reparent the component and tidy up the tree's state. */
>         if ( comp.parent != null) {
>         comp.parent.remove(comp);
>
> So that is exactly what i did type. Then we don't have any problems of
> duplicate childs.
>
> johan
>
>
>
> On 12/22/05, Juergen Donnerstag < [EMAIL PROTECTED]> wrote:
> >
> > > I don't think we can check if the component already has a parent and
> then just throw an
> > > exception.. Because it could be reusing components/panels and he wants
> to move if
> > > from X to Y
> >
> > And if we do parent = null during remove() and replace()?
> >
> > If we can not throw an exception, than add() is more like addOrMove(),
> > correct. Though it avoids the problem of having 2+ equal Component
> > objects in the hierarchy, I predict that users will still fall into
> > the trap and open bug reports. We will than close it with a comment
> > referring to the javadoc and we'll than do it a couple of times until
> > we evaluate a "better" solution.
> >
> > Indepent on how we decide, IMO we should make your change anyway.
> >
> > If removing the parent while removing the Component hierarchy does not
> > work. Another approach could be a transient map added to Page and each
> > object is put into it with its hashcode (kind of flattened tree). It
> > is fast and very easy to implement. And we do not need additonal
> > memory except for the HashMap. The components are already kept with
> > the hierarchy anyway. And because it is transient, it won't be
> > serialized. To rebuild the HashMap in case of a server switch in a
> > cluster is easy as well. What about this approach? But I'd still
> > prefer to remove the parent reference from the Component.
> >
> > Juergen
> >
> > On 12/22/05, Igor Vaynberg < [EMAIL PROTECTED]> wrote:
> > > that sounds good to me. it is something we need to have clearly
> documented
> > > in the add() function though.
> > >
> > > -Igor
> > >
> > >
> > >
> > > On 12/21/05, Johan Compagner < [EMAIL PROTECTED]> wrote:
> > > > Why don't we just do it as swing does?
> > > >
> > > > Check if the component already has a parent. And remove it from the
> parent
> > > first and then add it to yourself.
> > > >
> > > > I don't think we can check if the component already has a parent and
> then
> > > just throw an exception.. Because it could be reusing components/panels
> and
> > > he wants to move if from X to Y
> > > >
> > > > something like this in MarkupContainer:
> > > >
> > > >     private final void addedComponent(final Component component)
> > > >     {
> > > >         // Check for degenerate case
> > > >         if (component == this)
> > > >         {
> > > >             throw new
> IllegalArgumentException("Component
> > > can't be added to itself");
> > > >         }
> > > > <NEW>
> > > >         // remove if from another parent.
> > > >         MarkupContainer mc = component.getParent();
> > > >         if(mc != null)
> > > >         {
> > > >             mc.remove(component);
> > > >         }
> > > >         else if(mc == this)
> > > >         {
> > > >             // component already added don't do anything
> > > >             return;
> > > >         }
> > > > </NEW>
> > > >         // Set child's parent
> > > >         component.setParent(this);
> > > >
> > > >         // Tell the page a component was added
> > > >         final Page page = findPage();
> > > >         if (page != null)
> > > >         {
> > > >             page.componentAdded(component);
> > > >         }
> > > >     }
> > > >
> > > >
> > > > johan
> > > >
> > > >
> > > >
> > > >
> > > > On 12/22/05, Igor Vaynberg < [EMAIL PROTECTED]> wrote:
> > > > > what do you think about performance? seems kind of overkill to
> traverse
> > > the entire tree every time you add a component, especially for
> > > listview/dataview type stuff. i know it wont be a problem in prod
> because
> > > component-use-check should be turned off, but still in development it
> might
> > > be a pain.
> > > > >
> > > > > i think maybe this one we call user error and leave it at that?
> > > > >
> > > > > -Igor
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On 12/21/05, Juergen Donnerstag < [EMAIL PROTECTED] >
> wrote:
> > > > > > We do not check that a component (same object instance) is added
> twice
> > > > > > at different levels in the hierarchy.
> > > > > >
> > > > > > This test could be added to MarkupContainer.add() though.
> Something
> > > like
> > > > > >
> > > > > >                 // Check that the same component instance is not
> > > > > >                 // added more than once.
> > > > > >                 if
> > > (getApplicationSettings().getComponentUseCheck())
> > > > > >                 {
> > > > > >                         Page page = findPage();
> > > > > >                         if (page != null)
> > > > > >                         {
> > > > > >
> > > page.visitChildren(new IVisitor()
> > > > > >                                         {
> > > > > >
> > > public Object component(Component component)
> > > > > >                                                 {
> > > > > >
> > >   if (component.equals(child))
> > > > > >
> > >   {
> > > > > >
> > >           throw new WicketRuntimeException(
> > > > > >
> > >                           "You can not add the same
> > > Component twice: "
> > > > > >
> > >                           + child.toString());
> > > > > >
> > >   }
> > > > > >
> > >   return IVisitor.CONTINUE_TRAVERSAL;
> > > > > >                                                 }
> > > > > >                                         });
> > > > > >                         }
> > > > > >                 }
> > > > > >
> > > > > >
> > > > > > It seems to work. Any objections?
> > > > > >
> > > > > > Juergen
> > > > > >
> > > > > > On 12/21/05, Juergen Donnerstag < [EMAIL PROTECTED]>
> wrote:
> > > > > > > On 12/21/05, Igor Vaynberg < [EMAIL PROTECTED] > wrote:
> > > > > > > > hi Juergen,
> > > > > > > > could you please take a look at
> > > > > > > >
> > > > > > > > Bugs item #1375584, was opened at 2005-12-07 18:44
> > > > > > > > Message generated for change (Comment added) made by ivaynberg
> > > > > > > > You can respond by visiting:
> > > > > > > >
> > >
> https://sourceforge.net/tracker/?func=detail&atid=684975&aid=1375584&group_id=119783
> > > > > > > >
> > > > > > > > i think this is more your area then mine. this guy added the
> same
> > > component
> > > > > > > > to the page twice. once to a form and once to a feedbackborder
> > > which was
> > > > > > > > also added to the form. wicket didnt catch this.
> > > > > > > >
> > > > > > >
> > > > > > > I'll check it.
> > > > > > >
> > > > > > > Juergen
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > -------------------------------------------------------
> > > > > > This SF.net email is sponsored by: Splunk Inc. Do you grep through
> log
> > > files
> > > > > > for problems?  Stop!  Download the new AJAX search engine that
> makes
> > > > > > searching your log files as easy as surfing the  web.  DOWNLOAD
> > > SPLUNK!
> > > > > >
> http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > > > > > _______________________________________________
> > > > > > Wicket-user mailing list
> > > > > > Wicket-user@lists.sourceforge.net
> > > > > >
> > >
> https://lists.sourceforge.net/lists/listinfo/wicket-user
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> > for problems?  Stop!  Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
> > _______________________________________________
> > Wicket-user mailing list
> > Wicket-user@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/wicket-user
> >
>
>


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_idv37&alloc_id865&opclick
_______________________________________________
Wicket-user mailing list
Wicket-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-user

Reply via email to