2
var app = angular.module('app',['ui.bootstrap']);

app.controller("ListCtrl", function ($scope, $http) {

    $scope.submit = function () {
        $scope.loading = true;
        $scope.error = false;
        $http.get('http://www.omdbapi.com/?s=' + $scope.search + '&r=json')
               .then(function (res) {
                    var titles = [];
                    angular.forEach(res.data.Search, function(item){
                       $http.get('http://www.omdbapi.com/?t=' + item.Title + '&y=&plot=full&r=json').then(function(res){
                       if (res.data.Poster === "N/A") {
                         res.data.Poster = "http://placehold.it/350x450/FF6F59/FFFFFF&text=Image+not+Available!!";
                       }
                        titles.push(res.data); 
                      });  
                    });

                   $scope.movie = titles;
                   $scope.results = true;
                   $scope.error = false;
                   $scope.loading = false;


                   if (titles.length==0) {           // not working
                       $scope.results = false;
                       $scope.error = true;
                   }


               })

I have been tried several things like :

Object.getOwnPropertyNames(titles).length === 0) 
obj == null 

None of them seems to work...

2
  • 1
    any working sample?? Commented Apr 27, 2015 at 6:14
  • Can you debug and check if the length is really zero! Commented Apr 27, 2015 at 6:18

4 Answers 4

2

This is happening because of incorrect scope:

var titles = []; is defined inside the .then

and you are checking the length outside of .then

since titles is not available outside .then it would not work. (undefined.length==0)

Solution:

.then(function (res) {
                    var titles = [];
                    angular.forEach(res.data.Search, function(item){
                       $http.get('http://www.omdbapi.com/?t=' + item.Title + '&y=&plot=full&r=json').then(function(res){
                       if (res.data.Poster === "N/A") {
                         res.data.Poster = "http://placehold.it/350x450/FF6F59/FFFFFF&text=Image+not+Available!!";
                       }
                        titles.push(res.data); 
                      }); 
               $scope.movie = titles;
               $scope.results = true;
               $scope.error = false;
               $scope.loading = false;


               if (titles.length==0) {           // now this will work
                   $scope.results = false;
                   $scope.error = true;
               }  
    });//titles will not be available after this.
Sign up to request clarification or add additional context in comments.

Comments

0

$http.get() is async so the statement if (titles.length==0) { gets executed right away.

Have a counter to determine when all the Promises get resolved and then perform the check. Move the if statement inside the callback.

 var count = res.data.Search.length;

 angular.forEach(res.data.Search, function(item){
   $http.get('http://www.o....rest of code').then(function(res) {
      // rest of code

      titles.push(res.data); 
      if (!count-- && !titles.length) {
          $scope.results = false;
          $scope.error = true;
        }
      }
    });
 });

Comments

0

In your case, the check

titles.length 

will be executed before the

titles.push

because you use an asynchronous request which will return later. You need to wrap your statements into the answer-block of the request.

Comments

0

Just as an aside, but beneficial for my practice and your future help:

Part of the problem you were having was scope-management (JS-scope, not Angular $scope), part of it was concurrency-management, and part of it appeared to be plain old scope-formatting making it hard to see where all control blocks start and end (that gets miserable when it's not just if/else, but with callbacks/promises, too).

This is a small example of how you might consider tackling these problems, through a quick refactor of your issues:

function pluck (key) {
  return function pluckFrom(obj) { return obj[key]; };
}

angular.module("app", ["ui.bootstrap"]);

angular.moule("app").service("omdbService", ["$http", function ($http) {
  function getSearch (search) {
    var searching = $http.get("http://www.omdbapi.com/?s=" + search + "&r=json")
      .then(pluck("data"));
    return searching;
  }

  function getMovie (title) {
    var searching = $http.get("http://www.omdbapi.com/?t=" + title + "&y=&plot=full&r=json")
      .then(pluck("data"));
    return searching;
  }

  return {
    getSearch: getSearch,
    getMovie: getMovie,
    getPlaceholderPoster: function () { return "http://placehold.it/350x450/FF6F59/FFFFFF&text=Image+not+Available!!"; }
  };
}]);



angular.moule("app").controller("ListCtrl", ["$scope", "$q", "omdbService", function ($scope, $q, omdb) {
  function loadMovie (movie) {
    return omdb.getMovie(movie.Title)["catch"](function () { return undefined; });
  }
  function movieExists (movie) { return !!movie; }
  function updatePoster (movie) {
    movie.Poster = movie.Poster || omdb.getPlaceholderPoster();
    return movie;
  }
  function setResults (movies) {
    $scope.movie = movies; // $scope.movies, instead?
    $scope.results = true;
    $scope.error = false;
    $scope.loading = false;
  }

  function handleError () {
    $scope.results = false;
    $scope.error = true;
  }

  $scope.submit = function () {
    $scope.loading = true;
    $scope.error = false;

    omdb.getSearch($scope.search)
      .then(pluck("Search"))
      .then(function (movies) { return $q.all(movies.map(loadMovie)); })
      .then(function (movies) { return movies.filter(movieExists).map(updatePoster); })
      .then(setResults, handleError);
  };

}]);

There are 8000 valid ways of handling this, and everyone will see it a little differently.
This is also not really how I'd tackle it in production, but not too far off...

Moving all endpoint-calls out to a service which is responsible for those means that any controller in your system (with that module as a dependency) can access them.

Doing small things per function and letting Array.prototype methods do the iterating (IE8 can be shimmed if needed) means that each function is super-specific.

By wrapping the controller/service functions in arrays, and naming their dependencies, they're now minification friendly.

The body of submit() is less than 10 lines, and deals with all kinds of crazy async stuff, but I know that I've handled errors like one of the movies returning a 404 (my code should still fire, with the remaining movies, the code of others might not -- most code would either never trigger success or would fail all the way through the program, if the server threw an error for a movie). Now, I'm not checking that the server is sending the right kind of data for a "movie", but that's different.

2 Comments

I tried your code as it is. This is intialising everything in my code at the beginning only, like error notif, loading notif etc. If you want I can post the repo link for you to contribute.
I'll definitely have a look, over the next few days, and potentially submit a PR after that.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.