Mailing-List: contact oak-dev-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: oak-dev@jackrabbit.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAOFYJNZ-wEiJJVHBa5a4MdUajPs-u2MJ3ZBT1jk+47dXe45ahg@mail.gmail.com>
References: 
 <CAB+dfi=jdjni+L4uhFxvP-3g7OQmDxcYDu3v9cRuwyz7Rsj3ag@mail.gmail.com>
	<CAOFYJNZ-wEiJJVHBa5a4MdUajPs-u2MJ3ZBT1jk+47dXe45ahg@mail.gmail.com>
Date: Mon, 30 Sep 2013 20:08:09 -0700
Message-ID: 
 <CAB+dfikEvu6Nhq-Nfst9-5s1PZXGq1j0UgENSwothGE0D=WRAg@mail.gmail.com>
Subject: Re: Tree.hasChild() / Tree.getChild()
From: Tobias Bocanegra <tripod@apache.org>
To: oak-dev@jackrabbit.apache.org
Content-Type: text/plain; charset=UTF-8

Hi,

On Mon, Sep 30, 2013 at 6:56 PM, Jukka Zitting <jukka.zitting@gmail.com> wrote:
> Hi,
>
> On Mon, Sep 30, 2013 at 4:22 PM, Tobias Bocanegra <tripod@apache.org> wrote:
>> I was looking at some spots where the code can be optimized, and I
>> found some occurrences where a Tree.hasChild() is followed by a
>> Tree.getChild(). I think this pattern is used commonly.
>
> Right. I think the pattern dates back to the time before we added the
> Tree.exists() method and made Tree.getChild() return non-null every
> time regardless of whether the identified child exists or is
> accessible.
>
> Instead of the old pattern ...
>
>     if (tree.hasChild(name)) {
>         Tree child = tree.getChild(name);
>         ...;
>     } else {
>         ...;
>     }
>
> ... a better pattern would now be ...
>
>     Tree child = tree.getChild(name);
>     if (child.exists()) {
>         ...;
>     } else {
>         ...;
>     }
>
>> so I think that it could make sense to add an optimizable convenience method:
>> [...]
>>     @Nullable
>>     Tree Tree.getChild(@Nonnull String name, boolean mustExist)
>
> I'm not sure if this helps much beyond the exists() mechanism. The
> case of mustExists == false is equivalent to the existing getChild()
> method, and the access pattern with the mustExist == true case would
> be:
>
>     Tree child = tree.getChild(name, true);
>     if (child != null) {
>         ...;
>     } else {
>         ...;
>     }
>
> The only difference is between "child.exists()" and "child != null",
> and AFAICT any optimizations done in such an extra getChild() method
> could also be used to optimize the exists() method.

right. but depending on the persistence implementation and caching, it
could result in fetching the child twice if using just the exists()
check. It might be nit-picky, but each createChild() creates 2-3
objects. of course this optimization only makes sense, if that
signature is propagated further down.

OTOH, I looked at the places where hasChild/getChild is used again,
and it's only in 3-4 places. so maybe not important to enough to
change.

regards, toby