3 Replies - 292 Views - Last Post: 29 November 2019 - 02:56 AM

#1 tekblade   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 84
  • Joined: 21-December 17

How to manage asynchronous properly

Posted 28 November 2019 - 07:30 AM

Hi,
my code outputs everytime different numbers. Is this a proper way I am using it? Here is the code:

export class GetPlanetsService {
  url='https://swapi.co/api/planets/?page=';
  planets:Planet[]=[];
  headers: HttpHeaders = new HttpHeaders()
    .set('Accept', 'application/json');

  constructor(private http:HttpClient) { }


  getPlanet(pageIndex){                                          
    return this.http.get<Planets>(`${this.url}${pageIndex}`,{headers:this.headers});
  }
  getAllPlanets(){
    let numberOfPages=7;  // Tried to do it dynamically but got infinite loop
    for(let j=1;j<=numberOfPages;j++){
      this.getPlanet(j).subscribe(value=>{
        for(let i=0;i<value.results.length;i++){
          this.planets.push(value.results[i]);
          if(j==numberOfPages && i==(value.results.length-1)){
            console.log(this.planets);  //There is outputted everytime different number
          }
        }     

    });

    }
  } 



Have you got any tips and could you explain it in simple words? Regards

Is This A Good Question/Topic? 0
  • +

Replies To: How to manage asynchronous properly

#2 Ornstein   User is offline

  • New D.I.C Head

Reputation: 22
  • View blog
  • Posts: 43
  • Joined: 13-May 15

Re: How to manage asynchronous properly

Posted 28 November 2019 - 09:39 AM

I've not put a whole lot of thought into this, but at first glance I'm guessing your problem is that you're assuming (or your code assumes) the last request will finish after the others - which may not be the case (e.g. the 7th GET request may actually finish while the 3rd is still loading, etc).

Two possible approaches to solve this are:

1) Rewrite your code so that the next call is only started after the previous one is finished. You lose a degree of asynchronicity here, but it's perhaps easier to implement.

2) Rewrite your code to keep track of how many requests have finished (e.g. by updating a counter on each callback) and how many are expected to finish - and then when the expected number is reached, do whatever final cleanup/output/etc you want. (This would probably be the preferred solution - depending on your requirements)

Hopefully this gives you enough to work with.

This post has been edited by Ornstein: 28 November 2019 - 09:42 AM

Was This Post Helpful? 0
  • +
  • -

#3 baavgai   User is offline

  • Dreaming Coder
  • member icon


Reputation: 7505
  • View blog
  • Posts: 15,553
  • Joined: 16-October 07

Re: How to manage asynchronous properly

Posted 28 November 2019 - 06:20 PM

Looks like TypeScript and Angular. I'll meet you halfway with the TS.

Right, quick test:
// sorry, can't deal with all the angular cruft
import axios from "axios";

// note, didn't mess with any number or date translation... yet
interface Planet {
    name: string;
    rotation_period: string;
    orbital_period: string;
    diameter: string;
    climate: string;
    gravity: string;
    terrain: string;
    surface_water: string;
    population: string;
    residents: string[];
    films: string[];
    created: string;
    edited: string;
    url: string;
}

interface PlanetServiceResult {
    count: number;
    next: string;
    previous: string;
    results: Planet[];
}

const getPlanets = () => 
    axios.get<PlanetServiceResult>("https://swapi.co/api/planets/?page=").then(x => x.data);

getPlanets().then(x => console.log(x));



Results:
{ count: 61,
  next: 'https://swapi.co/api/planets/?page=2',
  previous: null,
  results:
   [ { name: 'Alderaan',
       rotation_period: '24',
       orbital_period: '364',
       ...



If I know how many I get per page, I could use count to know the number of pages. Or I could take advantage of that next and previous. I think I'll go with that.

const getPlanets = () => {
    const call = (acc: Planet[], url: string): Promise<Planet[]> =>
        axios.get<PlanetServiceResult>(url)
            .then(x => x.data)
            .then(x => x.next ? call([...acc, ...x.results], x.next) : [...acc, ...x.results]);
    return call([], "https://swapi.co/api/planets/?page=");
}

getPlanets()
    .then(xs => xs.map(x => x.name))
    .then(x => { console.log(x.length); console.log(x); });



Results:
61
[ 'Alderaan',
  'Yavin IV',
  'Hoth',
  'Dagobah',
  ...



Hmm, matched the count. Guessing mission successful.

Hope this helps.
Was This Post Helpful? 0
  • +
  • -

#4 baavgai   User is offline

  • Dreaming Coder
  • member icon


Reputation: 7505
  • View blog
  • Posts: 15,553
  • Joined: 16-October 07

Re: How to manage asynchronous properly

Posted 29 November 2019 - 02:56 AM

I felt moderately bad I left it like this, so...

I like promises. If you're working in ES6, Promise.all is your friend for this kind of thing. Since you know the number of pages, you can essentially do them all at once. I rather dislike Angular and haven't looked at it for some time. However, in the interests of a working example in context, I started here and found my way to a nice Stackblitz template.

I came up with this, that makes sense given what you're showing:
import { Injectable } from "@angular/core";
import { HttpClient } from "@angular/common/http";
import { PlanetServiceResult } from "./types";

@Injectable({
  providedIn: 'root'
})
export class PlanetService {
  constructor(private http: HttpClient) {}

  private getResult(page: number) {
    // const headers = new HttpHeaders().set('Accept', 'application/json');
    // {headers}
    return this.http.get<PlanetServiceResult>(
      `https://swapi.co/api/planets/?page=${page}`
    );
  }

  getPlanets() {
    return this.getResult(1).toPromise()
    .then(page1 => {
      // grab the first page, it has count and and a result set
      const { count, results } = page1;
      const pageSize = results.length;
      // from the size of that result set, we can figure out the number of pages
      const pages = Math.ceil(count / pageSize);
      // console.log({ count,  pages, pageSize }); // checking work as I go
      // I want calls for the remaining pages.  Here I'm just building an array of the page numbers
      const calls = (new Array(pages - 1).fill(0)).map((x, i) => i + 2);
      // now, I want to create an array of promise calls, feed it to Promise.all, and watch the magic happen
      return Promise.all(calls
        .map(page => this.getResult(page).toPromise()))
        // .then(xs => { console.log(xs); return xs; ) // check results
        // now I need to flatten the resulting array of full results into just the planet results
        .then(xs => xs.reduce((acc, res) => [...acc, ...res.results], page1.results) )
        // .then(xs => { console.log(xs); return xs; ) // check results
        // make it pretty and return it
        .then(xs => xs.sort((a,b ) => a.name.localeCompare(b.name)))
    })
  }
}



Some angular binding with above:
import { Component, OnInit } from "@angular/core";
import { PlanetService } from "./planet.service";
import { Planet } from "./types";

@Component({
  selector: "my-app",
  templateUrl: "./app.component.html",
  styleUrls: ["./app.component.css"]
})
export class AppComponent implements OnInit {
  name = "Angular";
  planets: Planet[];
  constructor(private svc: PlanetService) {}

  ngOnInit() {
    this.svc
      .getPlanets()
      // .then(x => { console.log(x); return x; })
      .then(res => (this.planets = res));
  }
}



You may find a live example here: https://stackblitz.c.../angular-spy9wk

To be honest, the Angular subscribe thing looks like a whole extra layer of framework abstraction, so I basically avoided it.

I'm afraid this might be too complete an answer, but it's really too complex to be coy. The take away should be that you can't push into arrays async and expect anything sane. Prefer promise all. Or my first recursive answer. Or...

Ok, back to sequential land and that loop you wanted to use. There is some syntax sugar for promises: await / async. This can make your async stuff look like sync stuff, for the most part.

So, something closer to what you had in mind might be:
  async getPlanets() {
    const page1 = await this.getResult(1).toPromise();
    const { count, results } = page1;
    let totalResults = [...results];
    const pageSize = results.length;
    const pages = Math.ceil(count / pageSize);
    for(let page=2; page <= pages; page++) {
      const res = await this.getResult(page).toPromise();
      totalResults.push(...res.results);
    }
    return totalResults.sort((a,B)/> => a.name.localeCompare(b.name));
  }



This is similar logic to the prior one, but more loopy. Saved here: https://stackblitz.c.../angular-px5cah
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1