If statement in function creates additional objects

Question

The function is supposed to add objects to a nested array(Items) in an array of objects and checks whether the object already exists. If it does exist it just increments the quantity, otherwise it should add a new item. However, once the nested array (Items) has a second object, it creates new objects instead of incrementing even if the condition (array[i].items[x].description == _description ) should return true. The _description and _quantity values are passed from user inputs.

var array = [];

AddCustomItem = function (_title, _description) {
if (array.length > 0) {
    for (var i in array) {
        if (array[i].RoomName === "Other") {                        
            for (var x in array[i].Items) {
                if (array[i].Items[x].description == _description) {
                    array[i].Items[x].quantity += parseInt(_quantity);
                    break;
                } else {
                    array[i].Items.push({
                        category : "Other",
                        description : _description,
                        quantity : parseInt(_quantity)
                    });
                    break;
                };
            };              
            break;  
        };
    };
} else  {
    array.push({
        RoomName : "Other",
        Items : [{
                category : "Other",
                description : _description,
                quantity : parseInt(_quantity)
            }
        ]
    });
};
var result = array;
return result;

};

You forgot to ask a question. What is your problem with the code, what does not work as expected? Please [edit] your post. — Bergi, Apr 20 '16 at 12:07

score 1 · Accepted Answer · edited May 23 '17 at 12:30

You're adding if the first entry of the array isn't a match, and not looking at any other entries in the array since you break; out of the array in both the if and else blocks. So the loop never loops. Instead, search the entire array, and then only add if you didn't find the entry.

Here's a minimal-changes version:

var found;
for (var x in array[i].Items) {
    if (array[i].Items[x].description == _description) {
        found = array[i].Items[x];
        break;
    }
}
if (found) {
    found.quantity += parseInt(_quantity);
} else {
    array[i].Items.push({
        category : "Other",
        description : _description,
        quantity : parseInt(_quantity)
    });
}

...but I'd probably use Array#find instead (you may need to shim it on older browsers):

var found = array[i].Items.find(function(item) { return item.description == _description; });
if (found) {
    found.quantity += parseInt(_quantity);
} else {
    array[i].Items.push({
        category : "Other",
        description : _description,
        quantity : parseInt(_quantity)
    });
}

Side note 1: for-in isn't usually a good way to loop through arrays. This answer describes your various choices for doing so.

Side note 2: The block attached to if, for, etc. doesn't need a ; after it. The block is already terminated by the }.

score 1 · Answer 2 · answered Apr 20 '16 at 12:13

However, once the nested array (Items) has a second object, it creates new objects instead of incrementing even if the condition (array[i].items[x].description == _description ) should return true.

That is because you have break after outer for and there is no second iteration it just adds element in else statement.

score 0 · Answer 3 · answered Apr 20 '16 at 12:10

it creates new objects instead of incrementing even if the condition (array[i].items[x].description == _description ) should return true

That is because this condition will not always return true. Since there will be other items in the array as well.

In your logic, it will go to else the first match doesn't happen.

You need to do

     var found = false;
     for (var x in array[i].Items) 
     {
            if (array[i].Items[x].description == _description) {
                array[i].Items[x].quantity += parseInt(_quantity);
                found = true;
                break;
            }
     }
     if (!found) {
                array[i].Items.push({
                    category : "Other",
                    description : _description,
                    quantity : parseInt(_quantity)
                });
                break;
     }

score 0 · Answer 4 · answered Apr 20 '16 at 12:12

Your issue is that on the first pass through your inner loop, you always either update the quantity or add a new item, and then you break out of the loop. You need to let it loop through all the items until either a match is found, or you run out of items. Then, if no match was found, you add an item. See my code below. It uses a boolean variable to indicate if a match was found or not.

var found = false;

for (var x in array[i].Items) {
  if (array[i].Items[x].description == _description) {
    array[i].Items[x].quantity += parseInt(_quantity);
    found = true;
    break;
  }
}

if (!found) {
  array[i].Items.push({
    category: "Other",
    description: _description,
    quantity: parseInt(_quantity)
  });
}

Bonus tips:

for loops and if else blocks don't need a semicolon after the closing }
your declaration of the AddCustomItem function will be a global variable, since you didn't declare it with var (unless you already declared the variable with var earlier)

If statement in function creates additional objects

4 Answers4