Comment regarding TreeImpl#remove

[Angela Schreiber <an...@adobe.com>]

hi michael

the implementation of Tree#remove starts with the following line:

         if (!isRoot() && parent.hasChild(name)) {

and it seems to me that testing for the parent containing the
Tree that i am having at hand is superfluous. is that the
leftover of a refactoring or could it really occur that in this
situation parent.hasChild returns false? if the latter was
true, i think this line deserves a little comment explaining
to the poor reader what are the circumstances of that behaviour.
maybe we would even need some log output for that special case.

what do you think?
angela

Re: Comment regarding TreeImpl#remove

[Angela Schreiber <an...@adobe.com>]

just found that Tree#getStatus() never returns REMOVED (see OAK-207).
if that was the reason for that extra check i would suggest to
refactor the code and check for status removed... that felt much
clearer to me.

gruesse
angela

On 7/24/12 1:30 PM, Angela Schreiber wrote:
> hi michael
>
> the implementation of Tree#remove starts with the following line:
>
>           if (!isRoot()&&  parent.hasChild(name)) {
>
> and it seems to me that testing for the parent containing the
> Tree that i am having at hand is superfluous. is that the
> leftover of a refactoring or could it really occur that in this
> situation parent.hasChild returns false? if the latter was
> true, i think this line deserves a little comment explaining
> to the poor reader what are the circumstances of that behaviour.
> maybe we would even need some log output for that special case.
>
> what do you think?
> angela

Re: Comment regarding TreeImpl#remove

[Michael Dürig <md...@apache.org>]

On 24.7.12 12:30, Angela Schreiber wrote:
> hi michael
>
> the implementation of Tree#remove starts with the following line:
>
>          if (!isRoot() && parent.hasChild(name)) {
>
> and it seems to me that testing for the parent containing the
> Tree that i am having at hand is superfluous. is that the
> leftover of a refactoring or could it really occur that in this
> situation parent.hasChild returns false? if the latter was
> true, i think this line deserves a little comment explaining
> to the poor reader what are the circumstances of that behaviour.
> maybe we would even need some log output for that special case.
>
> what do you think?

This is most probably a refactoring left over from rev. 1359334 and can 
be removed.

Michael

> angela