0

I have this method, which will search a LinkedList(named ListNode) and check for chars and check if they contains uppercase chars, then store then in a new linkedlist, and return that. I wrote the code for it, tested it with JUnit, but it failed JUNit(On of those blue boxes). Does anyone know what went wrong?

Here is my LinkedList method:

public static ListNode copyUpperCase(ListNode head) {
    ListNode newListNode = mkEmpty();
    if(head == null){
        throw new ListsException("");
    }else{      
        while(head.next != null){
            if(Character.isUpperCase(head.element)){
                newListNode.element = head.element;         
            }
            head = head.next;
        }
    }
    return newListNode;
}

Here is ListNode:

public class ListNode {
    public char element;
    public ListNode next;
}

And here is the test method:

@Test
public void testCopyUpperCase()
{
    // Inject upper case letters randomly in the test strings.
    // Assert equal results when extracting the upper case chars from
    // the corresponding list, as wheen extracting them from the 
    // injected string itself.
    for ( String s : cases ) {
        String uppersAndLowers = randInjectUpper(s);
        // Extract the upper case characters
        StringBuilder uppers = new StringBuilder();
        for ( int i = 0; i < uppersAndLowers.length(); i++ ) {
            final char c = uppersAndLowers.charAt(i);
            if ( Character.isUpperCase(c) )
                uppers.append(c);
        }
        ListNode temp = Lists.toList(uppersAndLowers);
        ListNode lhs = Lists.copyUpperCase(temp);
        assertFalse(hasSharedNodes(temp,lhs));
        ListNode rhs = Lists.toList(uppers.toString());
        assertTrue(Lists.equals(lhs,rhs));
    }
}

THe failed line in testmethod were the last, which is:

assertTrue(Lists.equals(lhs,rhs));

What does it mean, if it failed at that line?

ps. here is the equals method also:

// Two lists are equal if both are empty, or if they have equal lengths
// and contain pairwise equal elements at the same positions.
public static boolean equals(ListNode l1,ListNode l2) {
    if ( isEmpty(l1) && isEmpty(l2) )
        return true;
    else if ( isEmpty(l1) || isEmpty(l2) )
        return false;
    else { // both lists are non-empty
        ListNode p1 = l1.next, p2 = l2.next;
        while ( p1 != null && p2 != null ) {
            char c1 = p1.element, c2 = p2.element;
            if ( p1.element != p2.element )
                return false;
            p1 = p1.next;
            p2 = p2.next;
        }
        return p1 == null && p2 == null;
    }
}

EDIT: This is the new method:

public static ListNode copyUpperCase(ListNode head) {

    ListNode newListNode = mkEmpty();
    if(head == null){
        throw new ListsException("Lists: null passed to copyUpperCase");
    }else{
        String cpy = toString(head);
        char[] chry = cpy.toCharArray();
        for(int i = 0; i < chry.length ; i++ )
                if(Character.isUpperCase(chry[i])){
                    newListNode.element = chry[i];      
                }
                newListNode = newListNode.next;
        }           
    return newListNode;
}
UserFuser
  • 110
  • 12
  • i think the while loop in copyUpperCase if failing to copy the last item to the other list. – guido Apr 04 '14 at 11:41

2 Answers2

1

Your equals method seems to be ok. It correctly checks, whether both lists are empty, then it correctly checks, whether one is empty and the other not. Afterwards you iterate over both lists simultenously, checking each character, and at the end, you expect both pointers to be null. Everything ok.

So the problem must be in the toList method or in the copyUppercase method. And indeed you copyUppercase method is buggy. Ask yourself, what happens for a list with only one element? Afterwards ask yourself what happens to the last element of any provided list? Can you see the spot?

The while loop condition is wrong: It must simply be

while (head != null) { ... }
Seelenvirtuose
  • 20,273
  • 6
  • 37
  • 66
  • The problem must be in the copyUppercase. The toList method was provided by the professor, and copyUppercase is the the method im writing. I changed the while loop like you said, but I am still having a failure when testing. Im confused, I dont know what I should do. – UserFuser Apr 04 '14 at 12:16
  • I also see now, that you do not check the first element of the lists inside the equals method. This is also a bug (but does not refer to your current problem). You should write a print method for printing out all list elements. And then use this print method prior to called the seert method. With that you can see, whether the assert is correct or not. Additionally, you should learn how to debug. – Seelenvirtuose Apr 04 '14 at 12:50
  • I changed the code a little bit, I already have that method printing out all the elements, but as a string. check my edited post, i will put the new method at the bottom. @seelenvirtuose – UserFuser Apr 04 '14 at 14:15
  • Try to debug your code. And if you still don't see the problem (I don't), please ask it in a new question with mor code. – Seelenvirtuose Apr 04 '14 at 16:25
0

It means that lhs is not equal to rhs, according to the implementation in Lists.equals

The error appears to be that your copyUpperCase only returns a single element in the list given in the argument, or that your test code adds all the uppercase characters in the string to the rhs (depends what your intention was). But you may also have an error in your Lists.Equal method which is not shown.

PS. I advise you not to put random elements in your JUnit tests as it means that the results are not repeatable.

  • I have put the equals method in my post, please look at it and see if the problem may be there. – UserFuser Apr 04 '14 at 11:48
  • The equals method does not check the first elements in the list are the same, but otherwise looks ok, I'm making some assumptions about the rest of your code here of course. I suggest you find out how to use the [debugger](http://docs.oracle.com/javase/7/docs/technotes/tools/windows/jdb.html) and [printing diagnostic output](http://stackoverflow.com/questions/4005378/console-writeline-and-system-out-println). – Chris Thomson Apr 04 '14 at 11:55