Cant understand why my generic code is not working

Question

Below is a simple implementation of a linked list. I have just added the relevant code. First, I add some values to the list, 10,990 and 10000. When I am searching for the same values, I get true for key = 10, but false for key = 990 and key = 10000, though it should be true. Also, if I change the second value from 990 to 99 and search for key = 99, this time I am getting a true.

I am not sure about using generic type. I guess I am doing something wrong there. Because if I replace generic type with int, I get the correct behavior. Please suggest.

    public class LinkedListTest {
        public static void main(String[] args) {
            LinkedList<Integer> num = new LinkedList<Integer>();
            num.add(10);            
            num.add(990);
            num.add(10000);
            int key = 10; 
            System.out.println("Key " + key + " found ?" + num.findValue(key));
            key = 990; //also checked for Integer key = 990
            System.out.println("Key " + key + " found ?" + num.findValue(key));
            key = 10000;
            System.out.println("Key " + key + " found ?" + num.findValue(key));
        }
    }

    class LinkedList<T>{
        private Node<T> first;
        private class Node<T>{
           private T data;
           private Node<T> next;

           public Node(T data){
               this.data = data;
               this.next = next;
           }
       }

    public void add(T data){
        Node<T> nn = new Node<T>(data);
        nn.next = first;
        first = nn;
    }

    public boolean findValue(T key){
        Node current = first;
        while(current != null){
            if(current.data == key)
                return true;
            else
                current = current.next;
        }
        return false;
    }
}

do not compare objects with `==` that is general rule of thumb and issue in your code — user902383, Jul 11 '14 at 17:00
Just out of curiosity, why are you adding to the front of your linked list? You can certainly add elements however you like, but adding to the end seems like a more common pattern. — Hunter McMillen, Jul 11 '14 at 17:03
does it really deserve a downvote....considering that not using equals is a fundamental flaw,however a person may not know that Integer class caches Object values from -128 to 127(1 byte) — Kumar Abhinav, Jul 11 '14 at 17:07
ha ha...negative votes for asking questions!!...thanks to the person who gave that...and many thanks to others who helped me out. — ixxx686, Jul 11 '14 at 17:14
@drum community users should decide if the question deserves up or down votes. There's no specific rule to say that a question cannot be downvoted. — Luiggi Mendoza, Jul 11 '14 at 17:29
@AbhinavKumar the only reason to up or downvote a question/answer is by user opinion. There's no exact rule that states a question *deserves a downvote*. — Luiggi Mendoza, Jul 11 '14 at 17:30

score 7 · Accepted Answer · answered Jul 11 '14 at 17:00

The == operator compares two object references to see if they refer to the same object. With Integer values, the JVM will cache Integers from -128 through 127.

From the Integer.valueOf javadocs:

This method will always cache values in the range -128 to 127, inclusive, and may cache other values outside of this range.

When 10 and 99 are boxed, they result in the same Integer object (respectively) when another 10 and 99 are boxed. However, boxing non-cached Integer objects such as 990 and 10000 will result in different Integer objects each time.

Replace == with the equals method, to compare the key contents, not the key references.

if(current.data != null && current.data.equals(key))

score 2 · Answer 2 · answered Jul 11 '14 at 17:03

You should be using .equals() instead of == when checking if you've found the value you're looking for:

public boolean findValue(T key){
    Node current = first;
    while(current != null){
        if(current.data != null && current.data.equals(key))
            return true;
        else
            current = current.next;
    }
    return false;
}

When you declare your LinkedList as a list of Integers, your primitive int is wrapped in an Integer object before it is stored in the node. Thus == doesn't always work because you're not comparing two primitives.

thank you Cameron..the code works fine with the change u suggested. — ixxx686, Jul 11 '14 at 17:12

score 1 · Answer 3 · answered Jul 11 '14 at 17:00

1

Your problem is that you are using == instead of equals.
It works for int which is a primitive type, but for Integer (object) == returns true only if the two members are the same instance.

answered Jul 11 '14 at 17:00

Teovald

4,369
4
26
45

score 0 · Answer 4 · answered Jul 11 '14 at 17:01

0

In order to be "generic" you should use equals method instead of == operator.

answered Jul 11 '14 at 17:01

Gaskoin

2,469
13
22

Cant understand why my generic code is not working

4 Answers4