Problem with Linq

Hi

I dont know what the problem is but this function doesn't seem to be working right, I tested with debug.log on each line and even the first conditional (if (Inventory.Any(i => i.Name != item.Name))) doesn't get triggered, If I add the item (inventory.add(item)) directly without the conditionals it works so the function itself is working.

Code (CSharp):

1. public void AddItem(Item item){
2.         if (Inventory.Any(i => i.Name != item.Name)){
3.             Inventory.Add(item);
4.         }
5.         if (Inventory.Any(i=>i.Name == item.Name)){
6.         for(int t = 0; t < Inventory.Count; t++) {
7.         if (Inventory[t].Name == item.Name){
8.                 Inventory[t].Quantity += 1;
9.           return;
10.                      }
11.                 }
12.             }
13.      
14.     }

This is the line I use to call the function:

Code (CSharp):

1. Status.Instance.AddItem(ItemManager.Instance.ItemList[0]);

Any idea of what could be wrong?

 

Still need help with this!

 

If something is wrong first check your assumptions. I would create a single enumerator with a where to get all objects which contain the name and output this. so you see if there are objects or not.
next: you are querying your collection 3 times which is kind of pointless.
get all items with the same name. if there arent any add the item, for all returned items increase the count (should this only be one?).

i have no idea why your code does not work. at least one of the conditionals should be executed.
why do you use linq when you iterate your collection manually anyways?

 

The way I decided to layout my code has nothing to do with the problem, please stay on topic. I just ran several tests and no conditional of any type are being executed, but when I add the item with a simple .add() with no conditionals it works.

 

Trying something else could lead to a workaround or reveal some insights into the problem itself. But if my contribution annoys you that much I cease it. Sorry for "highjacking" your thread.

 

Dont be so precious about your code. You'll never learn anything. Exiguous makes valid points.

Linq should be avoided. While its nice, it has a tendency to create garbage for the GC. You are calling the same function twice which really isnt required since you intend to iterate the collection to update the quantity... so you are bascially iterating the collection 3 times for no gain.

A better solution...

Code (csharp):

1.  
2.  
3. public void AddItem(Item item)
4. {
5.   for(int t =0; t < Inventory.Count; t++){
6.     if(Inventory[t].name == item.name)
7.     {
8.        Inventory[t].quanity += 1;
9.        return;
10.     }
11.   }
12.  
13.   item.quantity = 1;
14.   Inventory.Add(item);
15. }
16.  
17.  

 

Again, I'm not being precious about my code, its just that's not what I asked, I had a problem which I solved myself while you guys where correcting my code, sure there are hundreds of more efficient ways to do something but first you need concentrate in the task at hand and fix the problem.

Now that I fixed the problem I can concentrate in making my code more efficient.

 

Well then do share. What caused the problem? So that we all know how to avoid it while still being able to write functions full of useless queries.

 

Here is JamesLeeNZ code fixed.

Code (CSharp):

1. public void AddItem(Item item){
2. if (Inventory.Count == 0){
3. item.quantity = 1;
4. Inventory.Add(item);
5. }
6.   for(int t =0; t < Inventory.Count; t++){
7.     if(Inventory[t].name == item.name){
8.        Inventory[t].quanity += 1;
9.        return;
10.     }
11.   }
12.   item.quantity = 1;
13.   Inventory.Add(item);
14. }
15.  

 

You're definitely going to get far with that attitude.

 

I always been nice to people here and always try to help them with their problems, I even shared all my creations (check my signature) for free, I don't have attitude problems I just wanted to stay focus on the problem which ultimately led me to a solution, if I have been perceived as rude it wasn't my intention at all. But hey, that is the past now, the problem has been solved.

 

You mean broken right?

That 'fixed' logic doesnt work correctly, unless you intend to Add an item, loop through the items, find it again, increment the quantity again...

My original solution without that additional check you added at the top was correct.

 

The first item you add to an empty Inventory will have a quantity of 2 now. The unmodified version was correct.