0

I am trying to create a Movie object from a Json output I got from an API. After each movie object is created, I add them an array of movies. All of this is inside a function and it returns the array of movies. When the function is called, I was able to console log the movies; however, when trying to get a specific item using an index, it returns undefined. Am I missing something in the code? Any help is greatly appreciated. Thanks!

function Movie(title, description, director, producer) {
  this.title = title;
  this.description = description;
  this.director = director;
  this.producer = producer;
}

var connectedToAPI = false;

function retrieveMovies() {

  var movies = [];

  // Create a request variable and assign a new XMLHttpRequest object to it.
  var request = new XMLHttpRequest();

  // Open a new connection, using the GET request on the URL endpoint
  request.open('GET', 'https://ghibliapi.herokuapp.com/films', true);

  request.onload = function() {
    // Begin accessing JSON data here

    var data = JSON.parse(this.response);

    if (request.status >= 200 && request.status < 400) {

      var x = 0;
      data.forEach(movie => {


        // var title = movie.title;
        // var description = movie.description;
        // var director = movie.director;
        // var producer = movie.producer;

        var film = new Movie(movie.title, movie.description, movie.director, movie.producer);
        movies[x] = film;
        x++;


      });

    } else {
      console.log('error');
    }


  }

  request.send();

  connectedToAPI = true;

  return movies;

}

var films = retrieveMovies();

if (connectedToAPI == true) {
  console.log(films);
  console.log(films.length);
  console.log("THIS IS MOVIE NUMBER 3: ");
  console.log(films[1]);
}

The console prints out:

[]    //->this contains the movies when expanded
0     //->the length is zero
THIS IS MOVIE NUMBER 3: 
undefined   //->retrieving specific item returns udefined
nem035
  • 34,790
  • 6
  • 87
  • 99
johnny87
  • 21
  • 1
  • 3

2 Answers2

1

Your code is running BEFORE the request has come back. This is known as a "Race condition". What you want is to pass a callback to your function or promise.

Put an argument in retrieveMovies like so

function retrieveMovies(callback)

Then pass the value of movies into your callback as an arg:

callback(movies)

But do that right after your forEach completes. Your main function won't have a value immediately.

Finally, before you call your api declare a variable onFinished and set it to a function that logs your results:

var onFinished = function(films){
  console.log(films);
  console.log(films.length);
  console.log("THIS IS MOVIE NUMBER 3: ");
  console.log(films[1]);
}
retrieveMovies(onFinished);

Note that you don't set the value of retrieveMovies or check for api connection state if you use this technique.

Also, you will note that you do not need to scope your movies array so far away from your loop. Or even at all...

The better technique would be to just invoke the callback with Array.map to avoid more clutter.

Simply invoke like this (delete your forEach and var movies entirely):

callback(data.map(m=> new Movie(m.title, m.description, m.director, m.producer));

A slick one-liner :)

Ron Gilchrist
  • 859
  • 1
  • 8
  • 21
  • 1
    Awesome! This solved my problem. And also, all the answers has hinted mo to read more about async function. I learned a lot! Thank you! – johnny87 Jan 21 '18 at 07:19
  • Cool, thumb my answer up! Read up on event loops in js, callbacks, promises, and bonus: higher order functions. Welcome to the wild wild world of js!! – Ron Gilchrist Jan 21 '18 at 07:21
  • Nice answer. But unless I misread the code, this isn't an example of a race condition, because the order of execution is known: the function will _always_ return before the asynchronous callback gets called. A race condition is a situation where the order of execution is unknown and may vary from run to run. An example is code that fires off requests to two different servers (or the same server) one right after the other, and the code assumes that the first request always responds before the second one. But the two responses may actually come back in either order. That is a race condition. – Michael Geary Jan 21 '18 at 07:24
  • Technically, you are correct. I use race condition fairly cavalierly. I am the type of guy to say "funner" ... because it is :). What is a better simple term for this (trying to synchronously find the value of an async function??) – Ron Gilchrist Jan 21 '18 at 07:29
  • Oh, I like "funner!" Made up words can be the best kind of words. I suppose of course that _every_ word was made up at some time or another. But I do think that when we use a technical term such as "race condition" it's best to stick with the actual definition to avoid confusion. In any case, keep up the good work! – Michael Geary Jan 21 '18 at 07:34
0

forEach is not modifying variable 'x'. It works on copies, producing undesirable effects in this case. Use a traditional for loop in cases when an index is to be accessed/used

gargkshitiz
  • 2,130
  • 17
  • 19