Doubly linked list of strings

Question

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;
 }
 }
}

Answer 1

There are a few things I see with your code that I would change:

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;

Answer 2

I would:

1. Rename DoublyLinkedList.current - saying things like current.next.setPrev(...) leads to confusion.
2. Remove DoublyLinkedList.traverse and all traverse related methods - traversal is an action taken against your list, and should not be a property of the list. As a result, a Node should only be able to return the previous or next Node (or null as appropriate). As a result, DoublyLinkedList.count can be removed as it's not required (maybe you meant to implement a size reference here?)
3. Rename DoublyLinkedList.first to DoublyLinkedList.head and implement the opposite, DoublyLinkedList.tail - and implement the obvious getters and setters to allow immediate traversal to either end of the List itself. I'd accept first and last as field names though, so your decision there.
4. Apply a code formatter of some description - sometimes you have an open brace before new line, sometimes the other way around. Formatting helps readability.
5. Rename DoublyLinkedList.s - s means nothing to anybody. Appropriate variable name for it's intended use.
6. Add another constructor to satisfy your requirement of specifying either previous or next nodes. Something like DoublyLinkedList(String strContent, Node prev, Node next) - implementing the necessary null checks isn't even required, just assign to your relevant fields. Note here that I've changed the parameter s to strContent - I don't like strContent but some deem the variable datatype as being useful within the variable name. I personally think that's an overflow from older IDEs.

You'll find that others on CodeReview will probably want you to add comments to your code. I disagree there - your code should be readable without comments. Your class has an appropriate name (DoublyLinkedList) therefore I expect to see certain fields and getters/setters. I don't expect to see business logic (such as traversal actions), but you might want to add a sort mechanism for convenience purposes.