How to avoid multiple subscribe which is used in other function in Angular?-CodePudding

Consider this code:

// search for a location and get its geo-coordinates
   searchLocation(location: string): void {
      this.gs
         .getLocationInfo(location)
         .pipe(
            tap((data: GoogleResponse) => {
               if (data.status === 'OK') {
                  this.lng = data.results[0].geometry.location.lng;
                  this.lat = data.results[0].geometry.location.lat;
                  this.name = data.results[0].name;
               }
            }),
            map((response) => response)
         )
         .subscribe((data: GoogleResponse) => {
            if (this.mode === 'tours') {
               this.getTours();
            } else if (this.mode === 'guides') {
               this.getLocalGuides();
            }
         });
   }

   getTours(): void {
      this.isLoading = true;
      this.ds.setLoadingStatus(this.isLoading);
      this.router.navigate(['/tours-view']);

      this.ts.getTourByPoiLocation(this.lng, this.lat).subscribe((tours: Tour[]) => {
         this.tours = tours;
         this.ds.setTours(this.tours);
         this.gs.setLocationName(this.name);
      });
   }

First I make an HTTP Request to Google using the Geocoordinate API to get the Geometry and save the data within the tap operator and then I'm using subscribe as this is an observable. (I also don't even use data: GoogleResponse in the subscribe.)

Within the subscribe I call the method getTours() to get some places saved in my DB with the latitude and longitude. I make another request to my server to retrieve the data. As this is an observable as well, I used subscribe again.

Everything works but I want to ask if there is any optimization to this code. I think I did some bad practices especially the subscribe in the subscribe.

Can I solve this with mergeMap or something?

CodePudding user response：

Yes, you can use mergeMap like this:

// search for a location and get its geo-coordinates
   searchLocation(location: string): void {
      this.gs
         .getLocationInfo(location)
         .pipe(
            tap((data: GoogleResponse) => {
               if (data.status === 'OK') {
                  this.lng = data.results[0].geometry.location.lng;
                  this.lat = data.results[0].geometry.location.lat;
                  this.name = data.results[0].name;
               }
            }),
            mergeMap((response: GoogleResponse) => this.ts.getTourByPoiLocation(this.lng, this.lat))
         )
         .subscribe((tours: Tour[]) => {
            if (this.mode === 'tours') {
               this.getTours(tours);
            } else if (this.mode === 'guides') {
               this.getLocalGuides();
            }
         });
   }

   getTours(tours: Tour[]): void {
      this.isLoading = true;
      this.ds.setLoadingStatus(this.isLoading);
      this.router.navigate(['/tours-view']);

      this.tours = tours;
      this.ds.setTours(this.tours);
      this.gs.setLocationName(this.name);

   }

Now you just have one subscribe

CodePudding user response：

Reading your question and your code it seems to me that you want to do the following:

get location info invoking getLocationInfo
once the response from getLocationInfo is received (more precisely, when the Observable returned by getLocationInfo emits), set some state attributes based on the values of the response received (if the condition data.status === 'OK' is true)
after this, decide (based on the mode value) whether to trigger the execution of the getTours or getLocalGuides method
in the case you trigger the execution of the getTours (i.e. if mode === 'tours'), you set some state, navigate to a new page and you trigger the execution of another Observable, the one returned by getTourByPoiLocation, and use the value emitted by this Observable to set some more state attributes with the response received
the function getTourByPoiLocation actually receives as input parameters properties of the value emitted by the Observable returned by getLocationInfo (in your code it uses the this.lng, this.lat which are set based on the value emitted by the Observable returned by getLocationInfo)
it is not clear what happens in case the method getLocalGuides is executed: this method can either execute another Observable or can run some standard synchronous logic; I assume it executes another Observable

If this understanding is right, I would try something along these lines

searchLocation(location: string): void {
  this.gs
    .getLocationInfo(location)
    .pipe(
       tap((data: GoogleResponse) => {
           if (data.status === 'OK') {
              this.lng = data.results[0].geometry.location.lng;
              this.lat = data.results[0].geometry.location.lat;
              this.name = data.results[0].name;
           }
        }),
        // use concatMap operator to execute the next Observable 
        // after the upstream Observable has notified something
        // We use the value emitted (of type GoogleResponse) to pass
        // the input parameters to the getTourByPoiLocation function
        concatMap((data: GoogleResponse) => {
          if (this.mode === 'tours') {
            this.isLoading = true;
            this.ds.setLoadingStatus(this.isLoading);
            this.router.navigate(['/tours-view']);
            // here we return the next Observable - concatMap requires, as
            // input, a function that returns an Observable
            return this.ts.getTourByPoiLocation(data.lng, data.lat)
             .pipe(
                tap((tours: Tour[]) => {
                  this.tours = tours;
                  this.ds.setTours(this.tours);
                  this.gs.setLocationName(this.name);
                })
             );
          }
          if (this.mode === 'guides') {
            // return the Observable that executes the logic for this case
            return functionThatReturnsTheObservableForGuides()
          }
        })
    )
    .subscribe();
}

You can factorize the code for the this.mode === 'tours' case in its own method, like this

getTours(lng, lat) {
  this.isLoading = true;
  this.ds.setLoadingStatus(this.isLoading);
  this.router.navigate(['/tours-view']);
  return this.ts.getTourByPoiLocation(data.lng, data.lat)
   .pipe(
     tap((tours: Tour[]) => {
       this.tours = tours;
       this.ds.setTours(this.tours);
       this.gs.setLocationName(this.name);
     })
   );
}

and end up with this version

searchLocation(location: string): void {
  this.gs
    .getLocationInfo(location)
    .pipe(
       tap((data: GoogleResponse) => {
           if (data.status === 'OK') {
              this.lng = data.results[0].geometry.location.lng;
              this.lat = data.results[0].geometry.location.lat;
              this.name = data.results[0].name;
           }
        }),
        // use concatMap operator to execute the next Observable 
        // after the upstream Observable has notified something
        // We use the value emitted (of type GoogleResponse) to pass
        // the input parameters to the getTourByPoiLocation function
        concatMap((data: GoogleResponse) => {
          if (this.mode === 'tours') {
            return getTours(data.lng, data.lat);
          }
          if (this.mode === 'guides') {
            // return the Observable that executes the logic for this case
            return functionThatReturnsTheObservableForGuides()
          }
        })
    )
    .subscribe();
}

I could not set up an equal example, so there may be syntactical errors, but I hope this give an idea about how to avoid nested subscriptions in this case.