2

I have a method that uses a service to get a list of books, i needed a second method that uses the list of books to get one book by filtering it by id. The two methods are called in "ngOnInit" of the same component. What i get is in the second method uses the books list before it gets its results.

I fixed the issue by calling the second method in the first inside the results of .subscribe, then calling only the first in ngOnInit. I didn't find the solution quite satisfying, i want a more general and organised way ...

export class BookComponent implements OnInit {
  public books: Book[];
  public book: Book;
  public id;
  public sub;

  constructor(private activatedRoute: ActivatedRoute, private router: Router, private bookService: BookService) {}
  getBooks() {
    this.bookService.getBooks().subscribe(
      result => {
        this.books = result;
      }
    );
  }

  findById() {
    this.sub = this.activatedRoute.paramMap.subscribe(params => {
      console.log(params);
      this.id = params.get('id');
      this.book = this.books.find(a => a.id == this.id);
    });
  }


  ngOnInit() {
    this.getbooks();
    this.findById();
  }
}

Here is the error i get:

 ERROR TypeError: Cannot read property 'find' of undefined
Rich
  • 1,567
  • 14
  • 24

3 Answers3

0

Try this

findById() {
this.sub = this.activatedRoute.paramMap.subscribe(params => {
  console.log(params);
  this.id = params.get('id');
  this.book = this.books.find(a => a.id == this.id);
});
}


ngOnInit() {
  this.books = [];
  this.getbooks();
  this.findById();
}
dota2pro
  • 7,220
  • 7
  • 44
  • 79
  • This prevents the undefined error, but does nothing for solving the underlying problem as to why it is undefined (or now `[]`). – Rich Jul 17 '19 at 17:53
  • Thanks foe the idea, it actually works but only on second try, the first try when i firstly route to one book and assign a specific "id" it doesn't work ... – e-ThimOverflow Jul 18 '19 at 07:44
0

This is happening because books wasn't initialized. Since you are running two async functions at the same time, there is no guarantee that the contents of books were initialized by getBooks() before you call findById().

You can avoid this issue by putting the call to findById() within the call to getBooks().This kind of leads to subscription hell (multiple nested subscriptions), but that can be cleaned up by using rxjs.

Rearranged your original code accordingly below.

export class BookComponent implements OnInit {
  public books: Book[];
  public book: Book;
  public id;
  public sub;

  constructor(private activatedRoute: ActivatedRoute, private router: Router, private bookService: BookService) {}
  ngOnInit() {
    this.getbooks();
  }

  getBooks() {
    this.bookService.getBooks().subscribe(
      result => {
        this.books = result;
        this.findById()
      }
    );
  }

  findById() {
    this.sub = this.activatedRoute.paramMap.subscribe(params => {
      console.log(params);
      this.id = params.get('id');
      this.book = this.books.find(a => a.id == this.id);
    });
  }

}
Rich
  • 1,567
  • 14
  • 24
  • Thank you for your reply, that's actually the solution i described, the one i disliked, because that way getBooks() doesn't only gets books but also aply the filtering by id ... That is why i wanted a more organised way. – e-ThimOverflow Jul 18 '19 at 07:40
  • You can use rxjs to do this, but it works differently than standard TS. See [this question/answer](https://stackoverflow.com/questions/45945846/how-to-make-2-dependent-http-request-in-angular-2) for more info. I'd say probably get the id from the params, then use that in your call to the http – Rich Jul 18 '19 at 12:59
0

Your instinct is correct that you should not be trying to call subscribes inside subscribes... You should be using higher order observables to accomplish this goal and make sure you get all of your observable data where it's needed. In this case, you want to use combineLatest:

ngOnInit() {
  this.sub = combineLatest(this.bookService.getBooks(), this.activatedRoute.paramMap)
               .subscribe(([books, params]) => }
                 this.books = books;
                 this.id = params.get('id');
                 this.book = this.books.find(a => a.id == this.id);
               });
}

now the subscribe callback will run everytime either observable stream emits, but only once both have emitted at least once. No need for the rest. In this case, I'm assuming getBooks() will only ever emit once, but the paramMap may emit again if the user navigates to a different book ID.

bryan60
  • 28,215
  • 4
  • 48
  • 65
  • Thank you for the well explained reply, i liked your idea, but also thought about how some of code lines will get repeated, because what i really want is a seperate method that gets me books, than re-use the results without re-getting them, maybe/potentially by other methods like findById ... – e-ThimOverflow Jul 18 '19 at 07:54