2

I am creating a generic class for a doubly-linked circular list with a dummy header node.

After searching StackOverflow, I have changed my class declarations (The list and the node) to extend Comparable and Cloneable as such:

public class DoublyLinkedList <T extends Comparable & Cloneable>{
private static class Node<T extends Comparable & Cloneable>
...

However, this line of code:

Node trav = new Node(l.current.data.clone());

Gives me this error:

DoublyLinkedList.java:43: error: clone() has protected access in Object
    Node trav = new Node((T)l.current.data.clone());
                                          ^

I have changed it to this:

Node trav = new Node((T)l.current.data.clone());

and the error remains. What am I missing? Thanks in advance.

Note: the Node constructor I'm calling looks like this:

    private Node (T d)
    {
        data=d;
        prev=null;
        next=null;
    }

Edit: Here is where the problematic code lies:

public DoublyLinkedList(DoublyLinkedList<T> l)
{
    size = l.size();
    this.head = new Node<T>(null);
    current = this.head;

    l.begin();
    while (!l.end()) {
        Node trav = new Node(((T) l.current.data).clone());
        current.next = trav;
        trav.prev = current;
        current = trav;
        l.advance();
    }
    current.next = this.head;
    this.head.prev = current;
}

Final Edit: Thanks to RealSkeptic, I have learned that two references to the same object makes no difference for immutable objects and shouldn't be the concern of a generic container for mutable objects, therefore I have decided to go with the following code:

Node trav = new Node(l.current.data);
beresfordt
  • 5,088
  • 10
  • 35
  • 43
Rvngizswt
  • 33
  • 5

2 Answers2

0

You need to implement clone() in the Object you are calling clone on. It also needs to be made public.

http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#clone%28%29

That said, making a copy constructor is a cleaner way to accomplish this. Something like this should work for you. 

public Node(Node n) {
   this.x = n.x;
   this.y = n.y;
}
Dodd10x
  • 3,344
  • 1
  • 18
  • 27
-1

I think what you need is this : Node trav = new Node((T)(l.current.data.clone()));

I guess the class type of l.current is Node, and the type of l.current.data is T. However, if you didn't override the clone() method, it will only call Object.clone(), and the return value is an instance of Object.

Explicit cast from class Object to T is definitely what you missed. Also check the clone() method in the class type T.

Peipei
  • 136
  • 1
  • 9
  • Type-casting to T as you suggested produced the same results. – Rvngizswt Apr 17 '15 at 19:23
  • 1
    No, `clone()` is `protected` in `Object`. No cast will do the job. It requires overriding - all the way through the hierarchy. – RealSkeptic Apr 17 '15 at 19:23
  • T is just some generic variable so that my list may be used on any type such as integers or Strings. Therefore is no way for me to change the code in T. Just avoid cloning and do a shallow copy then? – Rvngizswt Apr 17 '15 at 19:25
  • @NickMireles `Integer` and `String` are both immutable so there's no point in cloning them anyway. A shallow copy is the way forward. – Paul Boddington Apr 17 '15 at 19:27
  • @NickMireles Actually, `Integer` and `String` are immutable. There is no need to make copies of them, two references to them will not produce any undesirable side effects. But even for mutable object, if you want to build a generic container, leave the protective copies to the caller. – RealSkeptic Apr 17 '15 at 19:27