You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Matej Knopp <ma...@gmail.com> on 2007/04/01 00:03:17 UTC

Re: remove check on previous child in MarkupContainer#replace

I'm fine with either too.

-Matej

On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> That's an idea too. It's an extra method (not so nice) but more pricise (nice).
>
> I'm fine with either. What do others think?
>
> Eelco
>
>
> On 3/31/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > or add a replaceOrAdd() method
> >
> > -igor
> >
> >
> > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > >
> > > I'm backporting Wicket In Action's code currently, and stumbled upon
> > > something I really liked to be able to do in 2.0 which I can't do in
> > > the same way in 1.3.
> > >
> > > Consider this:
> > >
> > >   void setContentPanel() {
> > >     if (inEditMode) {
> > >       new DiscountsEditList(this, "content");
> > >     } else {
> > >       new DiscountsList(this, "content");
> > >     }
> > >   }
> > >
> > > I'd like to be able to write that as:
> > >
> > >   void setContentPanel() {
> > >     if (inEditMode) {
> > >       replace(new DiscountsEditList("content"));
> > >     } else {
> > >       replace(new DiscountsList("content"));
> > >     }
> > >   }
> > >
> > > However, there is this check in replace:
> > >
> > >                         // Add to map
> > >                         final Component replaced = put(child);
> > >
> > >                         // Look up to make sure it was already in the map
> > >                         if (replaced == null)
> > >                         {
> > >                                 throw new WicketRuntimeException(
> > >                                                 exceptionMessage("Cannot
> > > replace a component which has not been
> > > added: id='"
> > >                                                                 +
> > > child.getId() + "', component=" + child));
> > >                         }
> > >
> > > so I'd have to do something like:
> > >
> > >       if (get("content") != null) {
> > >         replace(new DiscountsEditList("content"));
> > >       } else {
> > >         add(new DiscountsEditList("content"));
> > >       }
> > >
> > > yech. I really don't see the added value of that check. I understand
> > > how it got in there, but when you think of it, what actual problem
> > > would this solve? Imo, we should just get rid of that check so that
> > > calling replace would be a simple add if the component wasn't added
> > > yet.
> > >
> > > Eelco
> > >
> >
>

Re: remove check on previous child in MarkupContainer#replace

Posted by Eelco Hillenius <ee...@gmail.com>.
It makes sense when you look at a component as a holder of a set of
components. However, I don't think put makes much sense when you think
about building a component tree.

Technically, put may be right, but I would say it's a little bit late
in the game to change add to put isn't it? And personally, I don't
think it sounds better nor is the more obvious choice. And having put
instead of replace and addOrReplace is only more confusing imo.

Eelco

On 4/1/07, Martijn Dashorst <ma...@gmail.com> wrote:
> To join in on the 3 letter lingo [1]: put() would be more accurate, as
> it resembles the contract for a Set.
>
> Martijn
>
> [1] http://en.wikipedia.org/wiki/Lingo_(game_show)
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: remove check on previous child in MarkupContainer#replace

Posted by Martijn Dashorst <ma...@gmail.com>.
To join in on the 3 letter lingo [1]: put() would be more accurate, as
it resembles the contract for a Set.

Martijn

[1] http://en.wikipedia.org/wiki/Lingo_(game_show)

-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: remove check on previous child in MarkupContainer#replace

Posted by Igor Vaynberg <ig...@gmail.com>.
then it would be more of a set() :)

-igor


On 3/31/07, Johan Compagner <jc...@gmail.com> wrote:
>
> if it was me just add()
>
> and should just replace if there is already one with that id if not then
> add.
> (and don't throw an error)
>
> thats how 2.0 really worked. (and for example Swing also)
>
> johan
>
>
> On 4/1/07, Igor Vaynberg <ig...@gmail.com> wrote:
> >
> > should actually be called addOrReplace() :)
> >
> > -igor
> >
> >
> > On 3/31/07, Matej Knopp <ma...@gmail.com> wrote:
> > >
> > > I'm fine with either too.
> > >
> > > -Matej
> > >
> > > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > > That's an idea too. It's an extra method (not so nice) but more
> > pricise
> > > (nice).
> > > >
> > > > I'm fine with either. What do others think?
> > > >
> > > > Eelco
> > > >
> > > >
> > > > On 3/31/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > > > or add a replaceOrAdd() method
> > > > >
> > > > > -igor
> > > > >
> > > > >
> > > > > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > > > >
> > > > > > I'm backporting Wicket In Action's code currently, and stumbled
> > upon
> > > > > > something I really liked to be able to do in 2.0 which I can't
> do
> > in
> > > > > > the same way in 1.3.
> > > > > >
> > > > > > Consider this:
> > > > > >
> > > > > >   void setContentPanel() {
> > > > > >     if (inEditMode) {
> > > > > >       new DiscountsEditList(this, "content");
> > > > > >     } else {
> > > > > >       new DiscountsList(this, "content");
> > > > > >     }
> > > > > >   }
> > > > > >
> > > > > > I'd like to be able to write that as:
> > > > > >
> > > > > >   void setContentPanel() {
> > > > > >     if (inEditMode) {
> > > > > >       replace(new DiscountsEditList("content"));
> > > > > >     } else {
> > > > > >       replace(new DiscountsList("content"));
> > > > > >     }
> > > > > >   }
> > > > > >
> > > > > > However, there is this check in replace:
> > > > > >
> > > > > >                         // Add to map
> > > > > >                         final Component replaced = put(child);
> > > > > >
> > > > > >                         // Look up to make sure it was already
> in
> > > the map
> > > > > >                         if (replaced == null)
> > > > > >                         {
> > > > > >                                 throw new
> WicketRuntimeException(
> > > > > >
> > > exceptionMessage("Cannot
> > > > > > replace a component which has not been
> > > > > > added: id='"
> > > > > >
> +
> > > > > > child.getId() + "', component=" + child));
> > > > > >                         }
> > > > > >
> > > > > > so I'd have to do something like:
> > > > > >
> > > > > >       if (get("content") != null) {
> > > > > >         replace(new DiscountsEditList("content"));
> > > > > >       } else {
> > > > > >         add(new DiscountsEditList("content"));
> > > > > >       }
> > > > > >
> > > > > > yech. I really don't see the added value of that check. I
> > understand
> > > > > > how it got in there, but when you think of it, what actual
> problem
> > > > > > would this solve? Imo, we should just get rid of that check so
> > that
> > > > > > calling replace would be a simple add if the component wasn't
> > added
> > > > > > yet.
> > > > > >
> > > > > > Eelco
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: remove check on previous child in MarkupContainer#replace

Posted by Johan Compagner <jc...@gmail.com>.
if it was me just add()

and should just replace if there is already one with that id if not then
add.
(and don't throw an error)

thats how 2.0 really worked. (and for example Swing also)

johan


On 4/1/07, Igor Vaynberg <ig...@gmail.com> wrote:
>
> should actually be called addOrReplace() :)
>
> -igor
>
>
> On 3/31/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > I'm fine with either too.
> >
> > -Matej
> >
> > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > That's an idea too. It's an extra method (not so nice) but more
> pricise
> > (nice).
> > >
> > > I'm fine with either. What do others think?
> > >
> > > Eelco
> > >
> > >
> > > On 3/31/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > > or add a replaceOrAdd() method
> > > >
> > > > -igor
> > > >
> > > >
> > > > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > > >
> > > > > I'm backporting Wicket In Action's code currently, and stumbled
> upon
> > > > > something I really liked to be able to do in 2.0 which I can't do
> in
> > > > > the same way in 1.3.
> > > > >
> > > > > Consider this:
> > > > >
> > > > >   void setContentPanel() {
> > > > >     if (inEditMode) {
> > > > >       new DiscountsEditList(this, "content");
> > > > >     } else {
> > > > >       new DiscountsList(this, "content");
> > > > >     }
> > > > >   }
> > > > >
> > > > > I'd like to be able to write that as:
> > > > >
> > > > >   void setContentPanel() {
> > > > >     if (inEditMode) {
> > > > >       replace(new DiscountsEditList("content"));
> > > > >     } else {
> > > > >       replace(new DiscountsList("content"));
> > > > >     }
> > > > >   }
> > > > >
> > > > > However, there is this check in replace:
> > > > >
> > > > >                         // Add to map
> > > > >                         final Component replaced = put(child);
> > > > >
> > > > >                         // Look up to make sure it was already in
> > the map
> > > > >                         if (replaced == null)
> > > > >                         {
> > > > >                                 throw new WicketRuntimeException(
> > > > >
> > exceptionMessage("Cannot
> > > > > replace a component which has not been
> > > > > added: id='"
> > > > >                                                                 +
> > > > > child.getId() + "', component=" + child));
> > > > >                         }
> > > > >
> > > > > so I'd have to do something like:
> > > > >
> > > > >       if (get("content") != null) {
> > > > >         replace(new DiscountsEditList("content"));
> > > > >       } else {
> > > > >         add(new DiscountsEditList("content"));
> > > > >       }
> > > > >
> > > > > yech. I really don't see the added value of that check. I
> understand
> > > > > how it got in there, but when you think of it, what actual problem
> > > > > would this solve? Imo, we should just get rid of that check so
> that
> > > > > calling replace would be a simple add if the component wasn't
> added
> > > > > yet.
> > > > >
> > > > > Eelco
> > > > >
> > > >
> > >
> >
>

Re: remove check on previous child in MarkupContainer#replace

Posted by Igor Vaynberg <ig...@gmail.com>.
should actually be called addOrReplace() :)

-igor


On 3/31/07, Matej Knopp <ma...@gmail.com> wrote:
>
> I'm fine with either too.
>
> -Matej
>
> On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > That's an idea too. It's an extra method (not so nice) but more pricise
> (nice).
> >
> > I'm fine with either. What do others think?
> >
> > Eelco
> >
> >
> > On 3/31/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > or add a replaceOrAdd() method
> > >
> > > -igor
> > >
> > >
> > > On 3/31/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > >
> > > > I'm backporting Wicket In Action's code currently, and stumbled upon
> > > > something I really liked to be able to do in 2.0 which I can't do in
> > > > the same way in 1.3.
> > > >
> > > > Consider this:
> > > >
> > > >   void setContentPanel() {
> > > >     if (inEditMode) {
> > > >       new DiscountsEditList(this, "content");
> > > >     } else {
> > > >       new DiscountsList(this, "content");
> > > >     }
> > > >   }
> > > >
> > > > I'd like to be able to write that as:
> > > >
> > > >   void setContentPanel() {
> > > >     if (inEditMode) {
> > > >       replace(new DiscountsEditList("content"));
> > > >     } else {
> > > >       replace(new DiscountsList("content"));
> > > >     }
> > > >   }
> > > >
> > > > However, there is this check in replace:
> > > >
> > > >                         // Add to map
> > > >                         final Component replaced = put(child);
> > > >
> > > >                         // Look up to make sure it was already in
> the map
> > > >                         if (replaced == null)
> > > >                         {
> > > >                                 throw new WicketRuntimeException(
> > > >
> exceptionMessage("Cannot
> > > > replace a component which has not been
> > > > added: id='"
> > > >                                                                 +
> > > > child.getId() + "', component=" + child));
> > > >                         }
> > > >
> > > > so I'd have to do something like:
> > > >
> > > >       if (get("content") != null) {
> > > >         replace(new DiscountsEditList("content"));
> > > >       } else {
> > > >         add(new DiscountsEditList("content"));
> > > >       }
> > > >
> > > > yech. I really don't see the added value of that check. I understand
> > > > how it got in there, but when you think of it, what actual problem
> > > > would this solve? Imo, we should just get rid of that check so that
> > > > calling replace would be a simple add if the component wasn't added
> > > > yet.
> > > >
> > > > Eelco
> > > >
> > >
> >
>