patternjavaMinor
Doubly linked list of strings
Viewed 0 times
listdoublylinkedstrings
Problem
This code works just fine as I get the correct output. However I want to make my code more simple. I want to avoid using the
setPrev(Node n) method and would rather set the reference to the previous node in the Node(String s) constructor directly somehow. I mainly want to know if this is the 'right' way to implement a Doubly linked list though.public class DoublyLinkedList {
private Node current;
private Node first;
private Node traverse;
private int count = 0;
public DoublyLinkedList(String s)
{
first = new Node(s);
current = first;
}
public void add(String s){
current.next = new Node(s);
current.next.setPrev(current);
current = current.next;
}
public void traverseForward(){
count++;
if(count==1)
traverse = first;
else
traverse = traverse.next;
System.out.println(traverse.toString());
}
public void traverseBackward(){
count--;
if(count < 1)
traverse = null;
else if(count == 1)
traverse = first;
else
traverse = traverse.prev;
System.out.println(traverse.toString());
}
private class Node{
private Node next;
private Node prev;
private String name;
public Node(String s)
{
this.name = s;
this.next = null;
}
public void setPrev(Node n){
this.prev = n;
}
public String toString(){
return this.name;
}
}
}Solution
There are a few things I see with your code that I would change:
Node API
First, I would make the members of class
LinkedList API
Instead of providing
All you need to keep track of in the list is the head of the list (and maybe the tail if you want O(1) insertion at the end of the list instead of O(n):
Node API
First, I would make the members of class
Node public. As it stands now, you only have one setter function to the prev member and that's it. This kind of defeats the purpose of making the entire class private to the DoublyLinkedList. Note that setting these members to be public doesn't break good encapsulation practice since the only class able to access and modify those members is the DoublyLinkedList. Moreover, why doesn't your constructor initialize prev? I would also add one or two more constructors that would allow me to construct a Node and assign the values of the next and prev references. This is how I'd revise class Node:private class Node{
public Node next;
public Node prev;
public String name;
public Node(String s)
{
name = s;
next = null;
prev = null;
}
public Node(String s, Node n, Node p)
{
name = s;
next = n;
prev = p;
}
public String toString(){
return this.name;
}
}LinkedList API
Instead of providing
traverseBackward() and traverseForward(), you may want to have your class implement Iterable and/or Iterator and provide an iterator interface to the user. Also, I would provide a size() or length() member function to return the size of the list. Otherwise, why bother having or keeping track of its size? I'd also throw in an insert(), is_empty(), clear(), and remove() function to the API. Moreover, this is very unnecessary:private Node current;
private Node first;
private Node traverse;All you need to keep track of in the list is the head of the list (and maybe the tail if you want O(1) insertion at the end of the list instead of O(n):
private Node head, tail;Code Snippets
private class Node{
public Node next;
public Node prev;
public String name;
public Node(String s)
{
name = s;
next = null;
prev = null;
}
public Node(String s, Node n, Node p)
{
name = s;
next = n;
prev = p;
}
public String toString(){
return this.name;
}
}private Node current;
private Node first;
private Node traverse;private Node head, tail;Context
StackExchange Code Review Q#125402, answer score: 3
Revisions (0)
No revisions yet.