[ 
https://issues.apache.org/jira/browse/JCR-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571397#action_12571397
 ] 

Stefan Guggisberg commented on JCR-1402:
----------------------------------------

[...]
>  > they're expected to operate on normalized paths.
>  Then the documentation of the relevant methods of the Path interface should 
> say so.
>  
>  I's suggest: remove "Note that there migth be an unexpected result if this 
> path is not normalized, e.g. the ancestor of degree = 1 of the path "../.." 
> would be ".." although this is not the parent of "../.."." and add 
> "IllegalArgumentException if this path is not normalized" to the javadoc of 
> getAncestor.

+1 for javadoc improvement. i am not sure about the IllegalArgumentException...
 
>  
> 
>  > it seems like the Path semantics got unfortunately somehow compromised
>  There are two different things which got mixed up: paths and the hierarchy 
> they denote. Some methods make that separation pretty clear: getDepth refers 
> to the hirarchy and getLength to the path. However with the 
> ancestor/descendant methods this separation is currently not that clear: 
> although one expects them to refer to the hierarchy the current default 
> implementation does suggest otherwise and the documentation is not clear 
> enough (see above).

the implementation suggesting otherwise is a bug, see JCR-1409 ;)

 IMO getAncestor clearly refers to the hierarchical nature of a path.

>  

> Path.getAncestor and Path.getAncestorCount are misnomers
> --------------------------------------------------------
>
>                 Key: JCR-1402
>                 URL: https://issues.apache.org/jira/browse/JCR-1402
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>    Affects Versions: 1.4
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: path.patch
>
>
> Although the method names refer to ancestors they operate on sub-paths. 
> Consider:
> PathFactory pf = PathFactoryImpl.getInstance();
> Path.Element p = pf.getParentElement();
> Path path = pf.create(new Path.Element[]{p, p});
> Path ancestor = path.getAncestor(1);
> assertFalse(ancestor.isAncestorOf(path) )  
> This is not what one would expect from looking an the method signatures. 
> I suggest to rename getAncestor to getSubPath, clarify the javadoc, and 
> deprecate getAncestorCount. 
> A patch follows.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to