patternjavaModerate
Circular linked list of integers
Viewed 0 times
integerslistcircularlinked
Problem
I'm looking for a review paying special attention as to whether the code can be optimized in the form of number of variables used, especially for the
```
public class circularLL {
Node start;
Node last;
circularLL()
{
start=null;
last=null;
}
class Node
{
int data;
Node next;
Node(int datanew)
{
data=datanew;
}
public void setNext(Node n)
{
next=n;
}
public Node getNext()
{
return next;
}
public void setData(int datanew)
{
data=datanew;
}
public int getData()
{
return data;
}
}
public void insert(int datanew)
{
Node p=new Node(datanew);
if(start==null)
{
start=p;
last=p;
p.setNext(p);
}
else
{
last.setNext(p);
last=last.getNext();
last.setNext(start);
}
}
public void delete(int datanew)
{
Node temp=start;
Node temp1=start;
Node previous=null;
Node previous1=null;
if(last.getData()==datanew)
{
while(temp1.getData()!=datanew)
{
previous1=temp1;
temp1=temp1.getNext();
}
previous1.setNext(temp.getNext());
last=previous1;
}
while(temp.getData()!=datanew)
{
previous=temp;
temp=temp.getNext();
}
previous.setNext(temp.getNext());
}
public void display() {
int count = 0;
if(start == null) {
System.out.println("\n List is empty !!");
} else {
Node temp = start;
while(temp.getNext() != last) {
System.out.println("count("+count+") , node value="+temp.getData());
count
delete() method.```
public class circularLL {
Node start;
Node last;
circularLL()
{
start=null;
last=null;
}
class Node
{
int data;
Node next;
Node(int datanew)
{
data=datanew;
}
public void setNext(Node n)
{
next=n;
}
public Node getNext()
{
return next;
}
public void setData(int datanew)
{
data=datanew;
}
public int getData()
{
return data;
}
}
public void insert(int datanew)
{
Node p=new Node(datanew);
if(start==null)
{
start=p;
last=p;
p.setNext(p);
}
else
{
last.setNext(p);
last=last.getNext();
last.setNext(start);
}
}
public void delete(int datanew)
{
Node temp=start;
Node temp1=start;
Node previous=null;
Node previous1=null;
if(last.getData()==datanew)
{
while(temp1.getData()!=datanew)
{
previous1=temp1;
temp1=temp1.getNext();
}
previous1.setNext(temp.getNext());
last=previous1;
}
while(temp.getData()!=datanew)
{
previous=temp;
temp=temp.getNext();
}
previous.setNext(temp.getNext());
}
public void display() {
int count = 0;
if(start == null) {
System.out.println("\n List is empty !!");
} else {
Node temp = start;
while(temp.getNext() != last) {
System.out.println("count("+count+") , node value="+temp.getData());
count
Solution
I'm not a big algorithms/data structures guy, but the only thing that caught my attention was the
Besides that, there are a few comments I have in general about the code. Most of the following is fairly standard code format type stuff, but I think it's worth saying it as many times as possible.
Formatting
It's fairly minor, it helps to make sure there's an empty line between methods.
Also, I know some feel differently, but my preference about operators is that they have spaces on either side.
Naming
In Java, the convention is that class names start with a capital letter, so
Besides that, names of methods, members, variables, etc should follow lowerCamelCase. So,
In
Etc
I don't have too much else to say, but I would consider breaking this code out into two or three files. I would make a new class with
Sorry I couldn't help out more with the efficiency/number of variables issue, but you don't seem to be doing anything too crazy, and like I said before, data structures and algorithms aren't my strong suit. Just thought I'd throw in my two cents.
delete function. Is there a reason that delete uses data to identify the node, as opposed to just taking in the Node object you want to delete? It seems like that could get hairy if two nodes have the same data (which happens in main).Besides that, there are a few comments I have in general about the code. Most of the following is fairly standard code format type stuff, but I think it's worth saying it as many times as possible.
Formatting
It's fairly minor, it helps to make sure there's an empty line between methods.
Also, I know some feel differently, but my preference about operators is that they have spaces on either side.
data=datanew vs data = datanew.Naming
In Java, the convention is that class names start with a capital letter, so
circularLL should be CircularLL. Even better, it should really be something more descriptive like CircularLinkedList.Besides that, names of methods, members, variables, etc should follow lowerCamelCase. So,
datanew becomes dataNew, but I would personally call it newData.In
delete, avoid variable names like temp1 and previous1. With the number of variables and how they're named, you're starting to enter mental-juggling territory. What I mean by that is that as I read through the code, I have to keep reading back through the code to remember what a variable does because its name isn't descriptive enough.Etc
I don't have too much else to say, but I would consider breaking this code out into two or three files. I would make a new class with
main to separate the linked list from what's using it, and I might put Node in its own class.Sorry I couldn't help out more with the efficiency/number of variables issue, but you don't seem to be doing anything too crazy, and like I said before, data structures and algorithms aren't my strong suit. Just thought I'd throw in my two cents.
Context
StackExchange Code Review Q#38876, answer score: 11
Revisions (0)
No revisions yet.