4

My method cannot pass the unit test. I stared on it for 5 hours in vain. Could somebody help me to see what's wrong with it?

PS: the getAllRelations() method in my code below is to separated the formatted inputs into arraylists of ArrayList of strings, for example, if I have a formatted input like this(I used it in my test case that couldn't pass)

String format = "John Doe , Mary Smith" + "\n" + "Brian William , John Doe" + "\n" + "Brian William ,Robert Andrew" + "\n" + "Mary Smith , Max Jackson";

In each line, the first person is the parent of the second person. the getAllRelations() method will split this formatted strings into arraylists that each of the list only contains the two name strings(without spaces after or before names) in each line as its elements. for example arraylist1 will be a list contains"John" and "Mary Smith".

Here is my methods that I could not figure out what's wrong, I want to used this method to check if two people share the same ancestor.

private boolean hasSameAncestor(String person1, String person2){
    ArrayList<ArrayList<String>> allRelations = allRelations();
    int i = 0;
    int j = 0;
    String name1 = person1;
    String name2 = person2;
    String parent1;
    String parent2;
    for(i = 0, parent1 = ""; i < allRelations.size(); i++){
        if(name1.equals(allRelations.get(i).get(1))){
            parent1 = allRelations.get(i).get(0);
            for(j = 0, name2 = person2, parent2 = ""; j < allRelations.size(); j++){
                if(name2.equals(allRelations.get(j).get(1))){
                    parent2 = allRelations.get(j).get(0);
                    if(parent2.equals(parent1)){
                        return true;
                    }
                    else{
                        name2 = parent2;
                        j = 0;
                    }
                }
            }
            name1 = parent1;
            i = 0;
        }
    }
    return false;
}

My test case that cannot pass is like this.

    @Test
    public void testHasSameAncestor()
    FamilyTree familyTree4 = new FamilyTree("John Doe , Mary Smith" + "\n" + "Brian William , John Doe" + "\n" + "Brian William ,Robert Andrew" + "\n" + "Mary Smith , Max Jackson");
    assertEquals(true, format.hasSameAncestor("Max Jackson", "Robert Andrew"));
}

I cannot figure out what's wrong with my function, could somebody help me? Thank you very much.

Code that could paste to eclipse for debugging help

package test;

import java.util.ArrayList;
import java.util.Arrays;


public class Test1 {

    String test;

    public Test1(String test){
        this.test = test;
    }


    private ArrayList<String> lineRelations(){
        int i;
        ArrayList<String> lineRelations = new ArrayList<String>();
        String[] lines = test.split("\n");
        for(i = 0; i < lines.length; i++){
            lineRelations.add(lines[i]);
        }
        return lineRelations;
    }

    private ArrayList<ArrayList<String>> allRelations(){
        int i;
        ArrayList<ArrayList<String>> allRelations = new ArrayList<ArrayList<String>>();
        ArrayList<String> lineRelations = lineRelations();
        for(i = 0; i < lineRelations.size(); i++){
            ArrayList<String> eachLine = new ArrayList<String>(Arrays.asList(lineRelations.get(i).split("\\s*,\\s*")));
            allRelations.add(eachLine);
        }
        return allRelations;
    }

    public boolean hasSameAncestor(String person1, String person2){
        ArrayList<ArrayList<String>> allRelations = allRelations();
        int i = 0;
        int j = 0;
        String name1 = person1;
        String name2 = person2;
        String parent1;
        String parent2;
        for(i = 0, parent1 = ""; i < allRelations.size(); i++){
            if(name1.equals(allRelations.get(i).get(1))){
                parent1 = allRelations.get(i).get(0);
                for(j = 0, name2 = person2, parent2 = ""; j < allRelations.size(); j++){
                    if(name2.equals(allRelations.get(j).get(1))){
                        parent2 = allRelations.get(j).get(0);
                        if(parent2.equals(parent1)){
                            return true;
                        }
                        else{
                            name2 = parent2;
                            j = 0;
                        }
                    }
                }
                name1 = parent1;
                i = 0;
            }
        }
        return false;
    }
}

test case

package test;
import static org.junit.Assert.*;
import test.Test1;

import org.junit.Test;


public class Test1Test {

    @Test
    public void testHasSameAncestor(){
        Test1 test1 = new Test1("John Doe , Mary Smith" + "\n" + "Brian William , John Doe" + "\n" + "Brian William ,Robert Andrew" + "\n" + "Mary Smith , Max Jackson");
        assertEquals(true, test1.hasSameAncestor("Max Jackson", "Robert Andrew"));
    }
}
Yusuf K.
  • 4,195
  • 1
  • 33
  • 69
user1835156
  • 41
  • 1
  • 4
  • What is wrong? Did you manage to run it? Did you get any error? Put some debug stmts and see the output – mtk Nov 19 '12 at 08:36
  • why are you so mad... I just want to get some help.. I ran it. And the unit test told me the actual result is false rather than expected true... the debug doesn't show anything meaningful to me however... so I cannot figure out why – user1835156 Nov 19 '12 at 08:41
  • Give allRelations() method or a sample data which return allRelations() maybe allRelations() method have some bugs? – Yusuf K. Nov 19 '12 at 08:42
  • the allRelations() method has no problem trust me. Because it is the helper function for several my other methods, which all past the unit test, actually this hasSameAncestor is a helper function for another method, I tried a lot of separated test and finally know this is the wrong method – user1835156 Nov 19 '12 at 08:44
  • If you want us to debug your code and find the solution you must give us a full runnable code. So I could copy paste to my eclipse and may find and help you :) – Yusuf K. Nov 19 '12 at 08:46
  • @ykartal I have all ready give my runnable code, could you help me debug it? thank you very much – user1835156 Nov 19 '12 at 08:54
  • I found your error on algorithm and I will post soon – Yusuf K. Nov 19 '12 at 09:12
  • oh really! thank you very much @ykartal I'm looking forward to your post : ) – user1835156 Nov 19 '12 at 09:20
  • Staring at code is not an efficient approach to debug. I've written up an alternative: http://www.patriciashanahan.com/debug – Patricia Shanahan Nov 19 '12 at 10:23

5 Answers5

2

First find the base ancestor of both person and then compare them.

Please check it :)

public boolean hasSameAncestor(String person1, String person2) {
        ArrayList<ArrayList<String>> allRelations = allRelations();
        int i = 0;
        String name1 = person1;
        String name2 = person2;
        String parent1;
        String parent2;

        //Find first person's ancestor
        for (i = 0, parent1 = ""; i < allRelations.size(); i++) {
            if (name1.equals(allRelations.get(i).get(1))) {
                parent1 = allRelations.get(i).get(0);
                name1 = parent1;
                i = -1; // because i will increase before start new loop
            }
        }

        //Find second person's ancestor
        for (i = 0, parent2 = ""; i < allRelations.size(); i++) {
            if (name2.equals(allRelations.get(i).get(1))) {
                parent2 = allRelations.get(i).get(0);
                name2 = parent2;
                i = -1;
            }
        }
        System.out.println(parent1);
        System.out.println(parent2);
        if (parent1.equals(parent2)) {
            return true;
        }
        return false;
    }

Best wishes.

Yusuf K.
  • 4,195
  • 1
  • 33
  • 69
1

First of all, the data structures that you are using are horrible for this kind of application. Instead of packing everything into a string and then splitting the string around and process it in a doubled-for-loop, you should construct an actual data structure for your family tree.

In informatics, Trees are the kind of structure you want to use for this task. A tree has two different kind of objects:

1) A Node, which has two children, that are also Nodes.
2) A Leaf, which has no children.

You could model your family tree using this nodes and then apply one of the numerous tree-algorithms which are known. (A similar question for this is Intersection of 2 binary search trees)

To be more specific: Each person in your model would have two other persons defined as their parents. A leaf is a person which has no (known) parents. You could then run an algorithm which computes the intersection of two binary trees. If the intersection is empty, they don't have a common ancestor.

Community
  • 1
  • 1
zhujik
  • 6,514
  • 2
  • 36
  • 43
  • the input format is given by my teacher...I myself really don't want to packing them into such string... but..homework.. – user1835156 Nov 19 '12 at 08:59
  • You could use the input string to construct your data structure once, i don't think that your teacher will complain about that (otherwise i would question the competence of that guy). – zhujik Nov 19 '12 at 09:12
1

Your inner loops start always with 1.

Make i = -1, j = -1 instead of 0 in the loops will solve.

Bo PENG
  • 86
  • 4
1

"Max Jackson" and "Robert Andrew" have different ancestors. "John Doe" and "Robert Andrew" share the same ancestors. Replace your condition as shown below and check

assertEquals(false, format.hasSameAncestor("Max Jackson", "Robert Andrew"));
assertEquals(true, format.hasSameAncestor("John Doe", "Robert Andrew"));
Shan
  • 521
  • 2
  • 8
  • 28
1

your relations go from right to left, right?

So Max is related to Mary and Mary to John. Robert is only related to Brian. He is not at the right hand side. But you want to check the other direction, too (?), so Brian is related to John as well and they both have the same ancestor. But this is very unusual.

Check this solution using a hashmap and recursive search with the relations from left (key) to right (value):

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;

public class Test {

    private String test;
    private HashMap<String, String> allRelations;
    private ArrayList<String> ancestors;
    public Test(String test){
        this.test = test;
        allRelations = allRelations();
        ancestors= new ArrayList<String>();
    }

    private ArrayList<String> lineRelations(){
        int i;
        ArrayList<String> lineRelations = new ArrayList<String>();
        String[] lines = test.split("\n");
        for(i = 0; i < lines.length; i++){
            lineRelations.add(lines[i]);
        }
        return lineRelations;
    }

    private HashMap<String, String> allRelations(){
        int i;
        HashMap<String, String> allRelations = new HashMap<String, String>();
        ArrayList<String> lineRelations = lineRelations();
        for(i = 0; i < lineRelations.size(); i++){
            allRelations.put(Arrays.asList(lineRelations.get(i).split("\\s*,\\s*")).get(0), Arrays.asList(lineRelations.get(i).split("\\s*,\\s*")).get(1));                      
        }
        return allRelations;
    }

    public boolean hasSameAncestor(String person1, String person2){
        if (allRelations.containsKey(person1)){
            if (ancestors.contains(allRelations.get(person1))){                                             
                if (allRelations.containsKey(person2)){
                    if (ancestors.contains(allRelations.get(person2))){
                        return true;
                    } else if (allRelations.containsKey(allRelations.get(person2))){
                        return hasSameAncestor(person1, allRelations.get(person2));
                    } else {
                        return false;
                    }
                } else {
                    return false;
                }
            } else {
                ancestors.add(allRelations.get(person1));
                if (allRelations.containsKey(allRelations.get(person1))){
                    return hasSameAncestor(allRelations.get(person1), person2);
                } else if (allRelations.containsKey(person2)) {
                    return hasSameAncestor(person1,allRelations.get(person2));
                } else {
                    return false;
                }
            }
        } else {
            return false;
        }       
    }
}

This returns true for

Test test1 = new Test("a1 , b1" + "\n" + "b1 , c1" + "\n" + "a2 , b2" + "\n" + "b2 , c1" + "\n" + "a3 , c3");
System.out.println(test1.hasSameAncestor("a1", "a2"));

and false for

System.out.println(test1.hasSameAncestor("a1", "a3"));

regards

pyr0
  • 377
  • 1
  • 14