A question regarding singly-linked lists.

[mmoski]

Here is some code. It reads in a line, then puts each token in that
line into a list node. The problem is that it seems that the next
token simply gets put into the data field of the head node. This is
causing the .next to NEVER equal null and crashes JCreator. This list
is implemented myself because this is my first time using a linked
list and I don't want to use any pre-defined list classes. Any
suggestions as to why the head node won't stay as the head node?

import java.util.Scanner; //import everything
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.IOException;

public class Parse {

public static void main (String[] args) throws IOException {

BufferedReader br = new BufferedReader( new
InputStreamReader(System.in));

Node head = null; // set up head node
Node prev = null; // set up a node to keep track of position
Node a = new Node(); //set up node that will hold data
String line = null;
String word = null;

while(((line = br.readLine()) != null) && (!line.equals(""))) { //
reads in a line
Scanner sc = new Scanner(line);

while(sc.hasNext()){ //scans line for tokens
word = sc.next();
a.data = word; //sets data to the next word
if(head == null){ //if there is no head create one and make
it's data field equal a
head = a;
}else{ //if there is a just a head and 0 or more other nodes
make the current node's next field point to a
prev.next = a;
}
prev = a; // set the 'keeping track of' node to equal a.
}
}

Node pointer = head;

while(pointer != null){ //test to see if the pointer node exists
System.out.println("Working " + pointer.data); //print out the
data of the pointer node
pointer = pointer.next;//set the pointer to the next node
}

}
}

class Node{
public Node next = null;
public String data;
}

Any help would be freakin' awesome. Thanks.

 

[Daniel Pitts]

Here is some code. It reads in a line, then puts each token in that
line into a list node. The problem is that it seems that the next
token simply gets put into the data field of the head node. This is
causing the .next to NEVER equal null and crashes JCreator. This list
is implemented myself because this is my first time using a linked
list and I don't want to use any pre-defined list classes. Any
suggestions as to why the head node won't stay as the head node?

import java.util.Scanner; //import everything
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.IOException;

public class Parse {

public static void main (String[] args) throws IOException {

BufferedReader br = new BufferedReader( new
InputStreamReader(System.in));

Node head = null; // set up head node
Node prev = null; // set up a node to keep track of position
Node a = new Node(); //set up node that will hold data
String line = null;
String word = null;

while(((line = br.readLine()) != null) && (!line.equals(""))) { //
reads in a line
Scanner sc = new Scanner(line);

while(sc.hasNext()){ //scans line for tokens
word = sc.next();
a.data = word; //sets data to the next word
if(head == null){ //if there is no head create one and make
it's data field equal a
head = a;
}else{ //if there is a just a head and 0 or more other nodes
make the current node's next field point to a
prev.next = a;
}
prev = a; // set the 'keeping track of' node to equal a.
}
}

Node pointer = head;

while(pointer != null){ //test to see if the pointer node exists
System.out.println("Working " + pointer.data); //print out the
data of the pointer node
pointer = pointer.next;//set the pointer to the next node
}

}

}

class Node{
public Node next = null;
public String data;

}

Any help would be freakin' awesome. Thanks.

I have two pieces of help for you:

How to fix your code:
You need put your "Node a = new Node()" inside your for loop.

You're reusing the same node every time (so you end up with head =
head.next = prev =prev.next (oops)

How to fix your post:
When posting code to usenet, try to make sure that your lines fit in
80 columns. You're // comments have wrapped to the next line, and if
someone tries to cut/paste that into a compiler, they'll get an
error. You can usually avoid this by using /* */ style comments.

Also, its more common in Java to write a comment to explain the
following line, not to comment at the end of the line. Although
that's not as steadfast of a rule. For example:

/* While we have something to look at
while (pointer != null) {
/* Print out the data */
System.out.println(pointer.data);
/* Set the pointer to the next node */
pointer = pointer.next;
}

One more thing... Although, many teachers expect you to comment
excessively, its generally better to comment on WHY you chose to do a
set of statements, than to explain what each statement does. If
you're repeating yourself, the comment is just line noise.

/* While something() returns true */
while (something()) {
}

I know how to read code, so the "While something() returns true"
comment is utterly unhelpful. More advanced programmers will document
their code by using meaningful names for everything, and breaking down
the process into multiple methods.

while (moreToProcess()) {
Processable processable = getNextThingToProcess();
processable.process();
}

Isn't that nicer than:

/* While there is more to process */
while (node != null) {
/* Get the next thing to process */
Processable processable = node.data;
/* Do the processing */
processable.execute();
}

Some people might disagree with me, but I think you'll find many
agree.

Hope this helps
Daniel.

 

[zzantozz]

Here is some code. It reads in a line, then puts each token in that
line into a list node. The problem is that it seems that the next
token simply gets put into the data field of the head node. This is
causing the .next to NEVER equal null and crashes JCreator. This list
is implemented myself because this is my first time using a linked
list and I don't want to use any pre-defined list classes. Any
suggestions as to why the head node won't stay as the head node?
[...]
Node a = new Node(); //set up node that will hold data

Your problem is that you only do this once. You have to realize that
when you use the "new" operator, you're actually creating a new
"thing"--a Node in this case. You need to ask yourself whether a
linked list would have just one node or many nodes. The way you're
doing it now, you create only one Node and you continually change
what's in it and link it back to itself.

To clarify, if you do this:
Node a = new Node();
Node b = a;
a.data = "foo";

Then b.data is also "foo" because both a and b are actually the same
Node. Assigning a to b on the second line doesn't put a copy of the
Node in b. It makes b point at the same thing that a points at in
memory. That means any change to a is also a change to b.

Try this instead:
Node a = new Node();
Node b = new Node();
a.data = "foo";

Now b's data still is null because you have *two* Nodes now and not
just one Node with two variables pointing at it.

So in your loop that reads individual words, you'd need to do
something like this:
1. Create a new Node to hold the new word
2. Attach the newly created Node to the end of the list
3. Point to the new Node as the end of the list

Hope that makes sense.

 

[Lew]

A number of people are discussing this same project of mmoski's over in a
thread entitled
"Still in while loop hell, Now with refined question!"

 

[Joshua Cranmer]

while(((line = br.readLine()) != null) && (!line.equals(""))) {

My recommendation to save you from parenthesis headaches:

while (!"".equals(line = br.readLine())) {

I would also follow the recommendation of Mr. Pitts in suggesting that
descriptive comments precede the actual code. If for no other reason, it
makes it bearable for those of us on 80x24 terminals.

while(sc.hasNext()){
word = sc.next();
a.data = word;
if(head == null){
head = a;
}else{
prev.next = a;
}
prev = a;
}

Here your code is both inconsistent in whitespace and wrong. You are
only ever using one node. Insert an "a = new Node();" before your first
access of a to fix your problems.

 

[Gordon Beaton]

while (!"".equals(line = br.readLine())) {

Consider that there may not be any empty lines in the file. Don't you
want the loop to stop at EOF, when readLine() returns null?

I can't imagine "".equals(null) returning anything but false.

Beware of bugs in the above code; I have only proved it correct, not
tried it. -- Donald E. Knuth

Indeed...

/gordon

--

 

[Patricia Shanahan]

Consider that there may not be any empty lines in the file. Don't you
want the loop to stop at EOF, when readLine() returns null?

I can't imagine "".equals(null) returning anything but false.

Even if there were some clever way to combine the two tests, I think
there is a code readability argument against doing so. There are two
conditions for leaving the loop, empty line and end of file. Shouldn't
the code say so?

Patricia

 

[Lew]

It won't.

null is distinct from all String values.

Even if there were some clever way to combine the two tests, I think
there is a code readability argument against doing so. There are two
conditions for leaving the loop, empty line and end of file. Shouldn't
the code say so?

Many people have a disturbing tendency to treat null as an in-band value
instead of properly as an out-of-band value. It's even weirder when parsing
the result of a method that really depends on null being out of band.

Occasionally it is valid to map null to an in-band value, but this is always a
mapping and Patricia's argument would remain valid; the code that maps null to
an in-band value should be explicit.

Literate programming principles argue to make checks for null explicit. For
another thing, that'll avoid the sort of mistake represented by the code
snippet /supra/.