You now have a header and trailer pseudo-node:
header <--> first <--> second <--> ... <--> last <--> trailer
Instead, you would have to connect the first and last in both directions.
.-> first <--> second <--> ... <--> last <-.
| |
'-------------------------------------------'
Alternatively, you could also merge header and trailer in one node, but this then is no "pure" circularly linked list, since you have to step over the header/trailer node on traversal.
.-> first <--> second <--> ... <--> last <--> header/trailer <-.
| |
'---------------------------------------------------------------'
Answer from Paŭlo Ebermann on Stack OverflowWhat are the advantages of using a Circular Doubly Linked List?
Are Circular Doubly Linked Lists thread-safe?
You now have a header and trailer pseudo-node:
header <--> first <--> second <--> ... <--> last <--> trailer
Instead, you would have to connect the first and last in both directions.
.-> first <--> second <--> ... <--> last <-.
| |
'-------------------------------------------'
Alternatively, you could also merge header and trailer in one node, but this then is no "pure" circularly linked list, since you have to step over the header/trailer node on traversal.
.-> first <--> second <--> ... <--> last <--> header/trailer <-.
| |
'---------------------------------------------------------------'
A circular list has no header, trailer, first nor last element. This forces special cases on you.
You can either have a pseudo element or not.
If you have a pseudo element, then you need to handle it as a special case in getNext and getPrevious as it must be skipped over, but you don't need to worry about what happens when the list is empty or if you are removing something important.
If you have no pseudo element, then there are no special cases for next and previous, but there is a special case for add when the list is empty and remove if you remove the node you are using to link into the list with.
Error handling
I would expect the following to:
- Write errors to
System.errrather thanSystem.out. - Better yet, throw a
java.lang.IllegalArgumentExceptionthat can be handled accordingly by the caller.
Aside from someone watching the output, they'd have no way to know a call to remove failed. There certainly wouldn't be any (convenient, idiomatic) way for the program to know it had failed.
public void remove(int index) { int counter = 1; int currentSize = size(); if (index > currentSize + 1 || index <= 0) { System.out.println("Invalid index."); return; }
Throwing an exception also simplifies, because the return is no longer necessary.
Indexing
You also seem to be indexing starting at 1 rather than 0. This is inconsistent with pretty much any other indexed data structure in Java or most other modern, mainstream programming language.
Iteration
Many operations (like printing a string representation) on your list your become easier if you implement an iterator over lists.
I think it is unfortunate that insert and remove call size. While I understand that you would want to reject a bad index early in the function, calling size gives these functions the property that inserting/removing cost time linear in the size of the list, not linear in the index as may be expected (or hoped for). The out-of-range-ness could be detected during the (inevitable) iteration up to the insertion/removal index.
Generally data structures and algorithms should not print errors to System.out (where it may be mixed awkwardly in the middle of other output, or be invisible in GUI applications, etc, anyway it's a mixing of concerns), but report them to the caller.
In removeByCorporation method , you are just setting the executive to null , but considering this to be a doubly linked list , dont you think you need to set the references of the previous and next executive , so that the doubly linked list doesn't break .
The trick is to make sure that, on EVERY operation, you update the item being changed and the two others which reference it (quite handily in a doubly linked list, the two which reference it are also the two it references).
Check each of your methods, and ensure that in each one you are updating 4 fields per change - two in the subject, and one each in the two that are linked from the subject.
Java style convention
One of the first thing you should do when working in a new language is to look at style convention. You used C++ convention and not the Java one. Class name should be PascalCase and method name should be camelCase. Unless it's a constant, you should not use _ (except in some precise case).
if(isEmpty()) { ... } else { ... }
Should be :
if(isEmpty()) {
...
} else {
...
}
The else should be on the same line that the last }.
Variable declaration
I always like to start the class with class and instance variables. It will let me know what my class is using. So private node<T> head; should be before the constructor.
I would suggest that you avoid using single letters as a variable name. Reading x and p does not help to know what it's used for. Try to use descriptive name, there is no or almost no length limitation, so be creative!
Documentation
I see that you have good comments for your public methods. I would suggest that you use JavaDoc. It will almost change nothing, just some syntax changes. Here is one example of a JavaDoc :
/**
* Here is the description of what the method is doing. Some specific things that will
* be helpful to the caller like {@link OtherClass#methodName()}
* @param aParam What your param is or used for in your method
* @return what your method is returning
* @see OtherClass
*/
As @tot2 pointed in comments, you can use tools that will generate html from your Javadoc. If you check the Java 7 html documentation, it's all been built with it. It's useful if you use an IDE as it can be easily accessible when you code. In Eclipse, if you put your mouse hover a method/class, it will show you the Javadoc in a well presented way. If you want more information I would suggest you visit this Oracle page.
Bracket
This is a personal choice, but I would suggest to always use brackets even if you only have one line and it's possible to omit brackets.
if(isEmpty()) return; else if(p == p.getNext()) { System.out.println(p.getKey()); }
if(isEmpty()) {
return;
} else if(p == p.getNext()) {
System.out.println(p.getKey());
}
This will "save" you time if you need to add a new line of code in the if part. It will also prevent so weird bug if someone try to add a line, but forget that there was no brackets.
While Marc-Andre already talked a lot about Java Style Conventions, here's my 2 cents on your implementation.
Inheriting:
You did create a List. In Java it's customary to implement Interfaces if you have classes with similar use-cases and methods.
public class list<T> {
This should/could be:
public class List<T> implements java.util.List<T> {
In fact
Naming:
I suggest you change the name of your implementation to avoid confustion. Java already comes with two lists ((interface)java.util.List and (class)java.awt.List), that are just named List, you don't really need to introduce a third one ;)
By the way, if you implement an interface you are required to Override the methods that are specified by it.
In the case of List that's quite a few, including but not restricted to:
public int size();
public boolean isEmpty();
public boolean contains(T item);
public void add(T item);
public void remove(T item);
When implementing them make sure to use the @Override annotation:
@Override
public void add(T item) {
Node<T> oldHead = this.head;
this.head = new Node<T>(item);
head.setNext(oldHead);
oldHead.setPrev(head);
size++;
}
Which brings me to my next point:
Hiding inner classes:
There is absolutely no need to show how your list works internally by exposing a node-class to all who got your List.
Instead in Java you can use a feature called 'Inner Classes'. It's quite simple:
public class MyList<T> implements List<T> {
private Node<T> head = null;
private int size = 0; //you could also use long when you expect more than 2150kk items
public MyList() {}
private static class Node<T> {
private final T value;
private Node<T> next = null;
private Node<T> prev = null;
protected Node<T>(T value) {
this.key = value;
}
T value() {
return this.value;
}
void setNext(Node<T> newNext) {
this.next = newNext;
}
void setPrev(Node<T> newPrev) {
this.prev = newPrev;
}
Node<T> getNext() {
return this.next;
}
Node<T> getPrev() {
return this.prev;
}
}
//Here goes the rest of MyList implementation
}
I am using a few tricks here:
The first thing you might probably notice is the keyword final. In case you haven't heard of it yet: The compiler enforces, that a final variable is only assigned once. This allows me to make an Instance of Node single-use only. If you want to change an item, you will have to replace it with a whole new node.
Beware this does not prevent changes to Objects themselves. It is very possible to do:
private final Map<String, String> demonstration = new HashMap<String, String>();
public void doSomething() {
demonstration.put("Demo", "Value");
}
The other thing is that I initialize next and previous not in the constructor. This is just my personal preference!
Additionally the inner class is static. This prevents access from the inner class to the outer one when using this. For more information have a look at this CR-Answer.
While we're at constructors / initialization:
Consistency:
One big issue I see with your code, is that you don't work consistently:
public Node() { this.setNext(null); this.setPrev(null); }
public List() { head = null; }
You use quite the mix here. In the Node constructor you use the setters - even with this keyword - to instantiate your Node, in your list you do it directly - and without this - on the field.
While both may be legal, I personally prefer using the middle way. As you have seen in the code above I usually initialize fields by using the this, but not the setter methods. In case of equal names it's required to use this so I usually put it everywhere.
Example no. 2:
node<T> f = head, b = head.getPrev();
node<T> b = head.getPrev(); b.getPrev().setNext(head);
here you have some statements on one and the same line, while throughout the rest of your code, you mostly place statements on separate lines.
Here I advise to wherever possible use the second approach. One Statement has one line and one line has one (or no) statement.