1

I have two problems with this code. 1) Only the last element of beerObjects is saved to the database. 2) There are n duplicates of the last element (n = beerObjects.length) saved to the database.

function addBeersToDatabase(beerObjects) {
for (i = 0; i < beerObjects.length; i++) {
    console.log(beerObjects[i].beerId);
    var currentBeer = beerObjects[i];

    // check if beer is already in database
    Beer.findOne({'beerId': currentBeer.beerId}, function(err, beer){
        if (err) {
            handleError(err);
        }
        if (beer) {
            // beer is already in database
        }
        else {
            // add new beer to database
            console.log(currentBeer.beerId);
            var newBeer = new Beer();
            newBeer.beerId = currentBeer.beerId;
            newBeer.name = currentBeer.name;
            newBeer.description = currentBeer.description;
            newBeer.abv = currentBeer.abv;
            newBeer.image = currentBeer.image;

            newBeer.save(function(err) {
                if (err) {
                    throw err;
                }
            });
        }
    });
}

}

I want to loop through each beer and save its info to the database. I used findOne to prevent duplicates but this is not working. The first console.log() statement prints each beer id but the seconds console.log() statement prints just the last beer id multiple times.

spatel95
  • 447
  • 1
  • 5
  • 10

1 Answers1

3

The issue here is that in the findOne callback - your beerId will always be set to the last beer in beerObjects, because the loop finishes before you get to your first callback - welcome to asynchronous javascript.

One remedy for this is to wrap your findOne code in an IFFE (Immediately Invoked Function Expression). This code will complete before moving on to the next beer from beerObject.

Here is some more info on IFFE

Stack Overflow on IFFE

I took a quick pass at the code, I believe this should work, but you may have to make some adjustments with the internal code...

for(var i = 0; i < beerObjects.length; i++) {
    console.log(beerObjects[i].beerId);
    //var currentBeer = beerObjects[i]; dont need this now
    (function (currentBeer) {
        Beer.findOne({ beerId: currentBeer},
            function(err, beer) {
                if(!err && !beer) {
                    var newBeer  = new Beer();
                    newBeer.beerId = currentBeer.beerId;
                    newBeer.name = currentBeer.name;
                    newBeer.description = currentBeer.description;
                    newBeer.abv = currentBeer.abv;
                    newBeer.image = currentBeer.image;
                    newBeer.save(function(err) {
                       // log your error here...
                    });
                } else if(!err) {
                    console.log("Beer is in the system");
                } else {
                    console.log("ERROR: " + err);
                }
            }
        );   
    })(beerObjects[i].beerId);
}
Community
  • 1
  • 1
omarjmh
  • 13,632
  • 6
  • 34
  • 42