Java is not assigning values to my variables correctly, with linked lists

Question

public void returnRental(Customer cust){
    Rental toDelete = null; //Rental to be removed from list.
    LinkedList<Video> toReturn = null; //List of videos to be added to inventory.

    //Find appropriate rental according to customer name. 
    for(int i = 0; i < rentals.size(); i++){
        if(cust.getName() == rentals.get(i).getRentee().getName()){
            toReturn = rentals.get(i).getRented();
            toDelete = rentals.get(i);
        }
    }

here is the snippet of code that is giving me problems. I've debugged it in eclipse quite a bit which ended up just confusing me more. It hits the if, and passes the condition. But once it gets to assigning values to "toReturn" it assigns it an empty list with size 0. Where as I check my rentals Linked list and the correct value are there, but for some reason it is not getting assigned to my variables correctly :( The same happens to "toDelete" but this isn't a list, it is one instance of my class Rental. (The linked list is a list of rentals, which contains a linked list of videos)

No errors are thrown... Its a little difficult to explain, if you need more information please let me know and i'll clarify.

I'm at a loss, possibly because I'm not iterating through my linked list correctly?

@pst.. You should use `equals()` method to compare two strings.. Else it will never go inside your if.. — Rohit Jain, Oct 01 '12 at 19:45
@RohitJain I am not disagreeing, however: "But once it gets to assigning values to "toReturn" it assigns it an empty list with size 0.". So either the OP is mistaken or the `if` *is* being executed (however sketchy the conditional may be). I have another hypothesis: the conditional **is** true .. **multiple times** and the "last" assignment sticks ;-) — , Oct 01 '12 at 19:45
I'm guessing you're just starting out in a programming class. Here's a tip that will serve you well in the future: whenever you find yourself saying "Java is not... ", you should be saying "**I** am not...". — Michael Myers, Oct 01 '12 at 19:45
@pst.. Add a `sysout` statement inside your `if` as the first statment.. and check whether it is getting printed.. — Rohit Jain, Oct 01 '12 at 19:46
toReturn is set to `rentals.get(i).getRented()`, which means that getRented() is returning an empty list. Please try debugging/showing that method. — tcooc, Oct 01 '12 at 19:48
@digitalfresh I would debug starting from the loop (this would for example show if it's not just a case of equals vs ==). — Denys Séguret, Oct 01 '12 at 19:49
I'm a little confused. rentals.get(i) returns a Rental object. Rental.getRented() returns a LinkedList — ajon, Oct 01 '12 at 19:50
This does not look like a == vs equals problem obvioulsy. See http://stackoverflow.com/questions/767372/java-string-equals-versus . Unless each string compared is made with new String(...) — Jonathan Drapeau, Oct 01 '12 at 19:56
Okay, I'm quite positive it's not the if statement. I tried using equals and i get the same thing. The reason I know that it passes the if is because I debugged it in Eclipse, when i step through it makes it inside the if. I wouldn't say it made it into the if if i wasn't certain :P — DrizztDoUrden, Oct 01 '12 at 20:50

Denys Séguret · Answer 1 · 2012-10-02T05:59:59.343

3

Replace

if (cust.getName() == rentals.get(i).getRentee().getName()){

by

if (cust.getName().equals(rentals.get(i).getRentee().getName())){

You can't compare strings with == (except if your algorithm can ensure this is the same instance, which is almost never the case).

But the missing equals is not the only bug. It may be inside getRented() or elsewhere (you don't show what you do with toReturn and toDelete, so it's not clear if you don't have problems here).

Now, to go on chasing your bugs, you should either

debug, and put a breakpoint in your loop to check the state of rentals.get(i) and the execution at this point
if you can't debug, put a lot of System.println, so that you know what you have...

edited Oct 02 '12 at 05:59

answered Oct 01 '12 at 19:44

Denys Séguret

372,613
87
782
758

However (and while this is an issue that should be fixed); "But once it gets to assigning values to "toReturn" it assigns it an empty list with size 0." seems odd .. – Oct 01 '12 at 19:44
4

`You can't compare strings with ==.` You can, and you should if you want to check if they reference the same object, but not if you want to compare their values as in this case. – MrLore Oct 01 '12 at 19:45
@MrLore I added the precision but I'm not sure it's not more confusing now for a new programmer... – Denys Séguret Oct 01 '12 at 19:47
Unless he's creating each String in his application with new String(...), using == will work as well as equals. See my comment in the question and the linked question. If I could I would downvote as this answer doesn't answer the question at all as the informations provided state the variable toReturn is not null, so the condition is matched. – Jonathan Drapeau Oct 01 '12 at 20:07

score 1 · Answer 2 · answered Oct 01 '12 at 19:55

1

Possibly, your if condition is being hit more than once. First of all, check if this is actually happening. If so, check your logic and determine if you want to stop at the first occurence or at the last (this case seems to be the latter).

If you want to stop at the first occurence, break the iteration:

for(int i = 0; i < rentals.size(); i++){
    if(cust.getName() == rentals.get(i).getRentee().getName()){
        toReturn = rentals.get(i).getRented();
        toDelete = rentals.get(i);
        break;
    }
}

answered Oct 01 '12 at 19:55

Fritz

9,987
4
30
49

`break` is a good addition for efficiency but I don't see how it would fix the bug. – Denys Séguret Oct 01 '12 at 19:56
If the condition is being hit more than once it'll save the last occurence. What if the first time the if is being hit the values are OK and in the second time they're overwritten by empty values? That's why I suggested the OP to check his particular list data. – Fritz Oct 01 '12 at 19:57
@user1712761 I see... Please, can you post the list you're using? That might shed some light as to why is this happening. – Fritz Oct 01 '12 at 20:57

score 1 · Accepted Answer · answered Oct 01 '12 at 20:03

I've upvoted dystroy's answer because incorrect string comparison is always wrong.

But because that would fail differently (customer names not matching rentee names), I'm wondering if your issue is really caused by either of the following:

a problem in getRented(); or
cust having a null name on call, which would match a Rentee with a null name.

Jimmy · Answer 4 · 2012-10-01T20:24:14.940

0

for(int i = 0; i < rentals.size(); i++){ 
    if(cust.getName().equals( rentals.get(i).getRentee().getName())){ 
        toReturn.addAll(rentals.get(i).getRented());  
         //assumming it returns the list of Video object
        toDelete = rentals.get(i); 
    } 
}

edited Oct 01 '12 at 20:24

answered Oct 01 '12 at 20:18

Jimmy

2,589
21
31

getRented returns a list of video, as he said in his question. Replace add by addAll. That still doesn't fix the problem with toDelete. – Jonathan Drapeau Oct 01 '12 at 20:20
@Jonathan : Thanks for pointing that out. I was kind of confused there, so left comments after the line. – Jimmy Oct 01 '12 at 20:25

Java is not assigning values to my variables correctly, with linked lists

4 Answers4