[ 
https://issues.apache.org/jira/browse/CALCITE-6698?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Stamatis Zampetakis updated CALCITE-6698:
-----------------------------------------
    Description: 
Than naming of the methods in conjunction with the following Javadoc comment 

{code:java}
 * <p>In addition to the usual set methods, there are methods to determine the
 * immediate parents and children of an element in the set, and method to find
 * all elements which have no parents or no children (i.e. "root" and "leaf"
 * elements).
{code}

led me to the *wrong* conclusion that they do the following:

* {{getNonChildren}} return all elements which have no children (leafs)
* {{getNonParents}} return all elements which have no parents (roots)

while they do the opposite. 

The name is subject to different interpretations so to avoid similar confusion 
in the future we should add some explicit documentation to the methods.

  was:
{{PartiallyOrderedSet#getNonChildren}} and 
{{PartiallyOrderedSet#getNonParents}} methods return wrong results.

According to the class Javadoc:
{code:java}
 * <p>In addition to the usual set methods, there are methods to determine the
 * immediate parents and children of an element in the set, and method to find
 * all elements which have no parents or no children (i.e. "root" and "leaf"
 * elements).
{code}
* {{getNonChildren}} should return all elements which have no children (leafs)
* {{getNonParents}} should return all elements which have no parents (roots)

Currently, the two methods return the opposite results. 

{code:java}
PartiallyOrderedSet<Integer> poset =
        new PartiallyOrderedSet<>((i,j) -> i <= j);
    poset.add(10);
    poset.add(20);
    poset.add(30);
    StringBuilder sb = new StringBuilder();
    poset.out(sb);
{code}

{noformat}
PartiallyOrderedSet size: 3 elements: {
  30 parents: [] children: [20]
  20 parents: [30] children: [10]
  10 parents: [20] children: []
}
{noformat}

{code:java}
poset.getNonChildren(); // should return 10 but it returns 30
poset.getNonParents(); // should return 30 but it returns 10
{code}

The current implementation also contradicts the results of 
getChildren/getParents methods:
{code:java}
poset.getChildren(30); // returns 20 so it contradicts getNonChildren()
poset.getParents(10); // returns 20 so it contradicts getNonParents() 
{code}


> Add Javadoc to PartiallyOrderedSet#getNonChildren and getNonParents to 
> clarify their behavior
> ---------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-6698
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6698
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Stamatis Zampetakis
>            Assignee: Stamatis Zampetakis
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.39.0
>
>
> Than naming of the methods in conjunction with the following Javadoc comment 
> {code:java}
>  * <p>In addition to the usual set methods, there are methods to determine the
>  * immediate parents and children of an element in the set, and method to find
>  * all elements which have no parents or no children (i.e. "root" and "leaf"
>  * elements).
> {code}
> led me to the *wrong* conclusion that they do the following:
> * {{getNonChildren}} return all elements which have no children (leafs)
> * {{getNonParents}} return all elements which have no parents (roots)
> while they do the opposite. 
> The name is subject to different interpretations so to avoid similar 
> confusion in the future we should add some explicit documentation to the 
> methods.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to