CODEWITHSUNDEEP
javascriptjsonparsing
germainelol
germainelol

Reputation: 3331

Unable to remove a JSON item with a null value

I'm receiving data from an API where there are 2 title and 2 author attributes (call them title_1 and title_2 for example). I only need one title or author to save to my database, so I regard an item as invalid if both title_1 and title_2 is empty.

So here's the problem, it seems to work for the author, but I always end up with one item still being saved despite title_1 being null and title_2 being "". I'm unsure why, but I think maybe there is an issue with how I am parsing the values?

So here is the JSON for item I mentioned:

"title_1": null,
"title_2": "",

As I said, it successfully splices authors with the same issues. First I am fetching the data (which is received as a string) and parsing it like this:

request.get(apiUrl, function(error, response, body) {
      if(!error && response.statusCode == 200) {
        var data = JSON.parse(body);
        callback(error, data.items);
      }
    });

Then I call removeInvalidItems(data); later on which does this:

function removeInvalidItems(data) {
  for (var i = 0; i < data.length; i++) {
    if ( !isValid(data[i].title) && !isValid(data[i].title_2) ) {
      data.splice(i, 1);
    }
    if ( !isValid(data[i].author) && !isValid(data[i].author_2) ) {
      data.splice(i, 1);
    }
  }
  return data;
};

It is checking for validity like this:

function isValid(attr) {
  if (attr === null || attr === '') {
    return false;
  } else return true;
};

Can anyone think of a reason why this item still keeps on passing through this isValid() function as valid?

Upvotes: 0

Views: 64

Answers (2)

Amit
Amit

Reputation: 46323

Your problem is that when you splice an array like you do (splice(i, 1)), your "current index" already points to the next element. Consider the next example. It's "supposed" to remove odd items from the array, but it creates a mess:

var arr = [1, 2, 3, 4, 5, 6];

for(var i = 0; i < arr.length; i++) {
  if(i % 2 == 1)
    arr.splice(i, 1);
}

alert(arr);

To fix that, when you splice, reduce i by 1:

function removeInvalidItems(data) {
  for (var i = 0; i < data.length; i++) {
    if ( !isValid(data[i].title) && !isValid(data[i].title_2) ) {
      data.splice(i--, 1);
    }
    if ( !isValid(data[i].author) && !isValid(data[i].author_2) ) {
      data.splice(i--, 1);
    }
  }
  return data;
};

As a side note, you can greatly simplify your isValid function:

function isValid(attr) {
  return attr instanceof String && attr.length > 0;
};

Upvotes: 1

Leo Nyx
Leo Nyx

Reputation: 696

When two invalid items follow each other, your loop does not check a second one of them. This happens because you modify an array while looping over it.

Try this in a console to see what's going on.

data = [10, 1, 2, 3, 20, 30];
for (var i = 0; i < data.length; i++) {
    console.log('checking i=', i, 'data[i]=', data[i]);
    if (data[i] < 5) {
        console.log('removing i=', i, 'data[i]=', data[i]);
        data.splice(i, 1);
    }
}
console.log('data left', data);

Instead of modifying array in place, you should probably just filter it.

Upvotes: 1

Related Questions