Linked list, is it removing the first node?

[Tiny_trixer]

Hi everybody! I'm currently working on an assignment for school. I'm creating a linked list (a line at the liqure store to be exact) and I'm a little stuck. Part of the code:

public void ekspederKunde() {
        System.out.println("Velkommen til polet! Vennligst vis gyldig legitimasjon.\n");

        if (fremst == null) { // is there anyone in line?
            System.out.println("Det er ingen i køen!");
        } // end if
        else if (fremst != null) { // if there is anyone in the line chech age
            if (alder >= alderMinst) { // if age is high enough
                System.out.println("Takk for besøket! Velkommen igjen!");
            } // end if
            else if (alder < alderMinst) { // if customer is to young
                System.out.println("DU ER IKKE GAMMEL NOK! Løp avgårde før vi varsler politiet!");
            } // end else if
            denne = fremst.neste; // Is this correct for deleting/removing the first in line? In english: current = first.next
            totaltKunder--; // counter to reduce total number of customers in line
        } // end if

        System.out.println();
    } // end metode for ekspedring av kunder

Now, this above here is a method I created to handle the first customer in line. The first if is checking if anyone i acctually in the line (fremst = first) and if there isn't anyone, it writes a message. My problem is that if I've had anyone in line, this check isn't working! It kinda keeps adding negative amounts of people in line and treating these as customers.

I think one of two things might be the problem. Either my delete function (see comment in code) isn't working, or the program can't reach the right "alder" in the else if checks. (alder = age, btw)

Now, I'm totally new to coding linked lists so I have no idea where my problem is. Any help would be greatly appreaciated since I have a deadline for delivering this in two days.

Just tell me if more of the code is needed.

[masijade]

deleting the first (and setting the "current") would be more like

fremst = fremst.neste;
denne = fremst;

because, otherwise, you are never changing the value of "fremst", and so, are never "deleting" anything.

[Tiny_trixer]

Hmmm. Yes, I see your point, but I tried that and it just gives me a java.lang.NullPointerException error.

I tried this instead:

if (fremst != null) { // as long as there is a customer in line
            if(fremst == denne){ // and if this one is the first in line
                fremst = fremst.neste; // the first gets replaced by the next?
                denne = fremst; // and the current is set to the first
            }
            else if(fremst.neste == null){ // if this is the last one in line
                fremst = sist = null; // then this is set to null?
            }
        }

And it kinda worked. But when I run the method posted in my first post again it just prints the stuff after the first if. (The if fremst == null part.) So it lookes to me like it thinks the program set every customer to null...!? I can't really see why. (I've commented what I think the lines do. Correct me if I'm wrong.)

I have anoter method that counts every customer and if I run that method after the code over, it looks like only one customer got removed. This assignment is giving me gray hairs...

[masijade]

Where and when do you get the NPE, and "denne" is irrelevant in this instance if you are allways concerned with the first item in the list, and I have no idea what "sist" is. If it gives you an NPE immediately (i.e. on the second "run" even if there are multiple items in the list), then you are not setting neste correctly.

IOW, don't "try that instead", go back to what I suggested and post where you are getting the NPE and "when".

[Tiny_trixer]

sist = last. IOW it's saying that if that item/customer was the last one both first and last is null. :)

But back on track. When I tried

fremst = fremst.neste;
        denne = fremst;

In the second run of the method I get the NPE in the first of the two lines above. But since you said that "denne" in this case doesn't apply if I only want to consider the first customer, maybe it isn't so strange that I get the error...?

I thought I needed more than those lines no matter what. I've been thinking and when the program reaches the last customer, don't I need a routine that says something like:
- If first.next is null
- first = last = null
Or someting like that...? (Do I need that last-part in a singly linked line?) That's why I tried that other one.

[masijade]

Then it is because fremst.neste is null on the first run, which probably means that you are not "adding" items to the list correctly. It also, of course, means fremst is null in the second run and so those statements should never be executed, as they take place (as far as I can see) in the portion of the if block where fremst cannot be null, so either the code is not what you've posted here, or you are mistaken in where the NPE took place. And neither "denne" nor "sist" apply here as they are not being used here.

[masijade]

I thought I needed more than those lines no matter what. I've been thinking and when the program reaches the last customer, don't I need a routine that says something like:
- If first.next is null
- first = last = null
Or someting like that...? (Do I need that last-part in a singly linked line?) That's why I tried that other one.

You already have

if (fremst == null)

which should catch that immediately in the next iteration. And since this assignment takes place in the block opposite that question, the NPE should never happen there (as long as the code originally posted is what is really being used).

[Tiny_trixer]

The code is my acctuall code. I don't learn anything if I don't post what I'm acctually working on. And seeing how I'm going to have to put a referance to this post in my report on the assignment I don't gain anything by not posting the right code.

Okey, my fremst is set in another method. Pseudocode that method says:
- if the first in line is null, fremst = new somethingNode(age)
- else fremst.addNode(denne)
- and a counter to keep track of how many gets put in line

Now. The addNode-thingy is directed to a class that handles all the customers as contents i nodes. (I'm no good at explaining but I think that's about right.) This method and those things I had one of our TA's check yesterday, and he said it worked fine. If I print the counter it also outputs the correct number of inputs.

And if fremst is null on the second run, shouldn't the program output what I put under

if(fremst == null)

?

[Tiny_trixer]

You already have

if (fremst == null)

which should catch that immediately in the next iteration. And since this assignment takes place in the block opposite that question, the NPE should never happen there (as long as the code originally posted is what is really being used).

But that part of the code has nothing to do with the removing part of the method? I thought I would get an error or warning or something if I just used:

fremst = fremst.neste;
denne = fremst;

since the first line then would try to set the next first in line to a customer who doesn't exist...?

[masijade]

But that part of the code has nothing to do with the removing part of the method? I thought I would get an error or warning or something if I just used:

fremst = fremst.neste;
denne = fremst;

since the first line then would try to set the next first in line to a customer who doesn't exist...?

Yes it does. Since, according to the code you posted, those "copy lines" take place in an "else block". So, if fremst is null it will never make it to those "copy lines", so it can never throw an NPE. Which is why I say that either the code is not the right code or the NPE is coming from somewhere else.

Also, there is no problem, whatsoever, in setting a reference null. You get an NPE when you then attempt to use that null reference (i.e. fremst.neste throws the NPE when fremst is null, but fremst = fremst.neste works just fun when fremst is not null, but fremst.neste is null).

[masijade]

Post the complete error statck trace for the NPE, as well as the code around the line number referenced in the NPE.

[Tiny_trixer]

Yes it does. Since, according to the code you posted, those "copy lines" take place in an "else block". So, if fremst is null it will never make it to those "copy lines", so it can never throw an NPE. Which is why I say that either the code is not the right code or the NPE is coming from somewhere else.

Hmm, okey! Well, then I learned something new today.

Also, there is no problem, whatsoever, in setting a reference null. You get an NPE when you then attempt to use that null reference (i.e. fremst.neste throws the NPE when fremst is null, but fremst = fremst.neste works just fun when fremst is not null, but fremst.neste is null).

Aha, that clears things up a bit! I did not know you could do that.

I think I'm going to have to look at the code-bit that places a customer in the neste-position. Maybe both me and the TA overlooked something.

I mean, if the error isn't in the code for removing the fremst-person, it should be where I put the person in, shouldn't it?

Thank you for helping me understand this better!

[Tiny_trixer]

Post the complete error statck trace for the NPE, as well as the code around the line number referenced in the NPE.

The output with the error:

Exception in thread "main" java.lang.NullPointerException
at oblig1polet.Polet.ekspederKunde(Polet.java:108)
at oblig1polet.Polet.menyValg(Polet.java:54)
at oblig1polet.Main.main(Main.java:27)
Java Result: 1

The code I come to when I press the topmost line:

fremst = fremst.neste;

(There is nothing more around this code. Above is the method I posted in the starting post, belowe a counter; totalt++ , and that's it.)

[masijade]

The output with the error:

The code I come to when I press the topmost line:

fremst = fremst.neste;

(There is nothing more around this code. Above is the method I posted in the starting post, belowe a counter; totalt++ , and that's it.)

Uhm, what do you mean "above is the method". This should be part of that method. I.E. you should be using this and "denne = fremst" instead of "denne = fremst.neste". Post that method again, as it is now.

[Tiny_trixer]

I was a little unclear there. Sorry. Here is the code as is now.

public void ekspederKunde() {
        System.out.println("Velkommen til polet! Vennligst vis gyldig legitimasjon.\n");

        if (fremst == null) {
            System.out.println("Det er ingen i køen!");
        } // end if
        else {
            if (fremst.getPerson().getAlder() >= alderMinst) {
                System.out.println("Takk for besøket! Velkommen igjen!");
            } // end if
            else if (fremst.getPerson().getAlder() < alderMinst) {
                System.out.println("DU ER IKKE GAMMEL NOK! Løp avgårde før vi varsler politiet!");
            } // end else if
        } // end else

        fremst = fremst.neste; //This is where the NPE points to

        totaltKunder--; 
    } // end metode

As you can see I've done some changes in other places as well, but that is because it retreived the wrong age. Now it's retrieving the correct age of each costumer from the Person class. I hope.

Not that many changes in other words.

[masijade]

And you now have the "fremst = fremst.neste" completely outside of the if statement, which is the reason you are getting the NPE. That, and the counter, of course, should still be inside the else portion if statement.

[Tiny_trixer]

Sometimes I feel so blond I could die....

But now I'm back at the point where it doesn't seem to find any next customer (after the first run I'm back at getting only the output from if fremst==null) and I'm again thinking about wheter the addNode(denne) is working or not.
(I described that one in pseudocode further up somewhere.)

I mean, now it obviously has a customer in the first place, and it acctually checks it correctly, removes it from the line, and finds no more customers.

[masijade]

Well, if the very first "fremst = fremst.neste" results in "fremst" being null, then, either, there is only one item in the list, or the items are not being added to the list properly, but I haven't seen that method, nor how it is being used, so I can't say.

[Tiny_trixer]

That's exactly what I meant. I add 3-4 customers in the line (and according to the counter it gets added) but then that removing-method only finds the first one.

I don't feel I can post the entire code since this is an assignment but heres parts of how the customers gets added (translated):

if (first== null) {
            first= new Node(name, age); // Here the first customer gets added
        } // end if
        else {
            first.addNode(current); // and here the rest is supposed to go
        } // end else

From Node-class (also translated):

public void addNode(Node newNode) {
        if (next== null) {
            newNode = neste;
        } // end if
        else {
            Node temp;

            while (next != null) {
                temp = next;
            } // end while
            next= newNode;
        } // end else

My guess is that the addNode isn't as it should be. But I asked the TA about this like I said, and he approved this part so I don't really know.

Oh, by the way. The constructor for Node-class:

public Node(String name, int age) {
        customer= new Person(name, age);

[masijade]

Well, shouldn't you be assigning newNode to neste, instead of the otherway around? (Hint, yes you should be).

Also, those "next" variables should be "neste", should they not?

You should also do Node temp = neste, rather than just Node temp and from that point on you should use temp.neste every where it currently uses next (as I assume you are traversing the list to find the end of it there), and that includes in the assignment at the end.

Edit: The thought behind the method is right, just skewed in the implementation. ;-)

[Tiny_trixer]

Well, it's good to know that I atleast thought right! ;)

And ofcourse those called next here is neste in the program. I might be blond, but I'm not platina. *lol*

Made the changes. Still getting the next first (if you catch my drift) as a null when I try the method the second time. I think I have to find someone who can help me look at it directly tomorrow.

But I think the removalpart of the method works now! At the very least. Thank you so much for all the help. I acctually did learn a lot!

[masijade]

Post your modified add method.

[Tiny_trixer]

Seeing how I'm running out of time with this assignment I made a really simple solution of it all and said:

if (fremst == null) {
            fremst = sist = new PersonNode(navn, alder); // adds the first one in line
        } // end if
        else {
            sist = sist.neste = new PersonNode(navn, alder); // adds everyone else
        } // end else

This of course is a much simpler solution that going through the add method but atleast this works. :P

I've still got some other stuff left to do before I can hand in the assignment but if I get time to spare I'm going to go back to the add method and see if I can make it work properly.

[masijade]

No, that should work just fine, since you are saving a "last" value.

[Tiny_trixer]

Yeah, atleast now I can run that method over and over until the list acctually is empty! So I'm happy!

Off to school now. Hopefully I'll be able to complete the assignment today now that I have a better understanding of how the linked list works.