Iterating with callback/anonymous functions

Question

I'm new to Node.JS and advanced Javascript in general, but I'm trying to build a schedule manager application on my own and I faced a problem (I will detail it later) when trying to execute the following code:

router.get('/', function (req, res) {
var day = new Date(req.query.day);
Location.getLocations(function (err, locations) {
    if (locations.length > 0) {
        var i;
        for (i = 0; i < locations.length; i++) {
            var location = locations[i];
            Appointment.getAppointments(day, location, function (err, appointments) {
                if (err) throw err;
                if (appointments.length == 0) {
                    // CREATE APPOINTMENTS
                    for (var j = location.available_time_start; j <= location.available_time_end; j += location.appointment_duration) {
                        var newAppointment = new Appointment();

                        newAppointment.start_date = new Date(day.getFullYear(), day.getMonth() + 1, day.getDate(), j);
                        newAppointment.appointment_duration = location.appointment_duration;
                        newAppointment.location = location.id;
                        newAppointment.booked = false;
                        newAppointment.locked = false;

                        Appointment.createAppointment(newAppointment, function (err, appointment) {
                            if (err) throw err;
                            console.log(appointment.location + ' - ' + appointment.start_date);
                        });
                    }
                }
            });
        }
    } else {
        // THERE ARE NO LOCATIONS
    }

    res.render('appointments', { locations: locations });
});

The problem is:
When I try to iterate the locations object and then execute the getAppointments function the code isn't executed at this exact moment. Later, when it's executed, location object is always the same (the iteration doesn't work), resulting on a unexpected result (all appointments with the same/last location).

I tried using IIFE (Immediately-invoked function expression) to execute the code instantly, but when I did this I couldn't get the appointments callback object and my logic is broken too.

Thanks in advance!

See [this answer](https://stackoverflow.com/a/19696026/5203655) for more info on callbacks and loops — Meghan, Apr 29 '18 at 05:07
Actually I viewed this before, but it doesn't seems to apply to my case. Could you please be more specific? — Adalberto Ferreira, Apr 29 '18 at 05:15
Thanks, it works! Coincidentally I was reading about `let` on this post when you commented: https://hacks.mozilla.org/2015/07/es6-in-depth-let-and-const/ — Adalberto Ferreira, Apr 29 '18 at 05:24
Possible duplicate of [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Snow, Jul 04 '19 at 08:47

score 0 · Accepted Answer · edited Apr 29 '18 at 06:05

0

The problem was solved by using let instead of var as suggested by @JaromandaX.

edited Apr 29 '18 at 06:05

31piy

23,323
6
47
67

answered Apr 29 '18 at 05:26

Adalberto Ferreira

9
5

HMR · Answer 2 · 2018-04-29T06:17:42.807

Your code seems to be saving appointments but doesn't do anything with the saved appointments (are you mutating locations?).

When saving an appointment goes wrong the requestor doesn't know about it because createAppointment is asynchronous and by the time the callback is called back res.render('appointments', { locations: locations }); is already executed.

You could try converting your callback based functions to promises:

const asPromise = (fn,...args) =>
  new Promise(
    (resolve,reject)=>
      fn.apply(undefined,
        args.concat(//assuming one value to resole
          (err,result)=>(err)?reject(err):resolve(result)
        )
      )
  );
const savedAppointmentsForLocation = (day,location,appointments) => {
  const savedAppointments = [];
  if (appointments.length == 0) {
    // CREATE APPOINTMENTS
    for (var j = location.available_time_start; j <= location.available_time_end; j += location.appointment_duration) {
      var newAppointment = new Appointment();
      newAppointment.start_date = new Date(day.getFullYear(), day.getMonth() + 1, day.getDate(), j);
      newAppointment.appointment_duration = location.appointment_duration;
      newAppointment.location = location.id;
      newAppointment.booked = false;
      newAppointment.locked = false;
      savedAppointments.push(
        asPromise(
          Appointment.createAppointment.bind(Appointment),
          newAppointment
        )
      );
    }
  }
  //you are not doing anything with the result of the saved appointment
  //  I'll save it as promise to see if something went wrong to indicate
  //  to the requestor of the api that something went wrong
  return Promise.all(savedAppointments);
}

router.get('/', function (req, res) {
  var day = new Date(req.query.day);
  asPromise(Location.getLocations.bind(Location))
  .then(
    locations=>
      promise.all(
        locations.map(
          location=>
            asPromise(Appointment.getAppointments.bind(Appointment),[day,location])
            .then(appointments=>[location,appointments])
        )
      )
  )
  .then(
    results=>//results should be [ [location,[appointment,appointment]],...]
      Promise.all(
        results.map(
          ([location,appointments])=>
            savedAppointmentsForLocation(day,location,appointments)
            .then(ignoredSavedAppointment=>location)
        )
      )
  )
  .then(locations=>res.render('appointments', { locations: locations }))
  .catch(
    error=>{
      console.log("something went wrong:",error);
      res.status(500).send("Error in code");
    }
  )
});

After your comment I studied a lot about promises and refactored my code to use them. Now I have a lot more control with the solution. Thanks! — Adalberto Ferreira, May 01 '18 at 07:12

Iterating with callback/anonymous functions

2 Answers2