CODEWITHSUNDEEP
angulartypescripthttprxjsobservable
awild
awild

Reputation: 49

Is There a More Efficient Way to Have a Subscription in a Subscription in Typescript

I am writing some code, in Typescript for Angular 11, where I need to get data from one observable and then looping through that have another call to get the name. Once I get the name I want to append it to the original object.

For example I have two observables and getSelfPurchases() returns:

{
  id: 32
  user_id: 5
  script_id: 78
  pp_id: "76SDI89768UIHUIH8976"
},
{
  id: 33
  user_id: 4
  script_id: 79
  pp_id: "78FGGF78SDF78FSD"
}

and the second one, getScriptDetails(32), returns:

{
  sname: "Spell Checker"
  author: 43
  price: 20.99
}

I have successfully achieved what I wanted to do but I feel like it is sloppy and inefficient. I've been reading more into RXJS stuff like switch map, but I am unsure if something like that can be done. Or maybe the method I chose is the best one already. Input?

this.userService.getSelfPurchases().subscribe(response => { // first observable
  this.purchases = response;

  this.purchases.forEach((purchase, index) => { // loop through our results
    this.scriptService.getScriptDetails(purchase.script_id).subscribe(responseTwo => { // second observable
      if (responseTwo[0] && responseTwo[0].sname !== undefined) {
        this.purchases[index].sname = responseTwo[0].sname; // append to our original purchases object
      }
    });
  });
});

Upvotes: 1

Views: 1905

Answers (3)

Mrk Sef
Mrk Sef

Reputation: 8022

You basically never want to nest subscriptions. It's not a matter of efficiency so much as it's a matter so much as maintainability, extendability, and (most importantly) readability.

Nested subscriptions quickly lead to call-back hell. It's surprisingly simple to get hopelessly lost that way. Though there's a time and place for everything I suppose.

This is your code re-written 1-1 as you had it before, without nesting subscriptions. I map your array of purchases into an array of getScriptDetails calls and then subscribe to that array via merge.

this.userService.getSelfPurchases().pipe(
  tap(response => this.purchases = response),
  map(purchases => purchases.map((purchase, index) => 
    this.scriptService.getScriptDetails(purchase.script_id).pipe(
      map(responseTwo => ({index, responseTwo}))
    )
  )),
  mergeMap(scriptDetailsCalls => merge(...scriptDetailsCalls)),
).subscribe(({index, responseTwo}) => {
  if (responseTwo[0] && responseTwo[0].sname !== undefined) {
    // append to our original purchases object
    this.purchases[index].sname = responseTwo[0].sname; 
  }
});

You can combine the map and mergeMap above into a single mergeMap as follows:

this.userService.getSelfPurchases().pipe(
  tap(response => this.purchases = response),
  mergeMap(purchases => merge(...
    purchases.map((purchase, index) => 
      this.scriptService.getScriptDetails(purchase.script_id).pipe(
        map(responseTwo => ({index, responseTwo}))
      )
    ))
  )
).subscribe(({index, responseTwo}) => {
  if (responseTwo[0] && responseTwo[0].sname !== undefined) {
    // append to our original purchases object
    this.purchases[index].sname = responseTwo[0].sname; 
  }
});

Aside: Avoid global variables

It's a personal taste for functional "purity," but it's generally cleaner to avoid the pattern where you set a global variable and then modify it later. Makes testing it harder as it leaves you with fewer guarantees about the state of that global variable.

this.userService.getSelfPurchases().pipe(
  mergeMap(purchases => forkJoin(
    purchases.map(purchase => 
      this.scriptService.getScriptDetails(purchase.script_id).pipe(
        map(responseTwo => ({...purchase, sname: responseTwo[0].sname}))
      )
    )
  ))
).subscribe(purchasesWName =>
  this.purchases = purchasesWName
);

Upvotes: 1

Eliseo
Eliseo

Reputation: 57919

it's a typical case swichMap, forkJoin, map

  1. First get the list
  2. Create an array of observables
  3. make the forkJoin
  4. map the initial list adding the values received

In code

this.userService.getSelfPurchases().pipe(
   switchMap(purchases=>{
         //here you has the purchases, e.g. [{script_id:2,script_id:4}]
         //so create an array with the observables
         const obs=purchases.map(purchase=>this.scriptService.getScriptDetails(purchase.script_id))
        //using switchmap we should return an observable
        return forkJoin(obs).pipe(
            //data is an array with the response of the call for script_id:2 and script_id4
            //but we don't want return only an array with the data
            //so we use map to transform the data
            map((data:any[])=>{
               //we loop over purchases to add the properties
               //we get in data
               purchases.forEach((purchase,index)=>{
                  purchase.sname=data[index].sname
                  purchase.author=data[index].author
                  purchase.price=data[index].price
                  purchase.author=data[index].author
               }
               //finally return purchases
               return purchases
            })
        )
   })
)

Upvotes: 1

Guntram
Guntram

Reputation: 980

Subscriptions should not be nested.

There are possibilities to e.g. concat, forkJoin, switchMap or merge pipes for combining subscriptions.

Nesting subscriptions is an anti pattern: https://www.thinktecture.com/de/angular/rxjs-antipattern-1-nested-subs/

RxJs Observables nested subscriptions?

Upvotes: 0

Related Questions