CODEWITHSUNDEEP
angularrxjs
Shamshiel
Shamshiel

Reputation: 2211

RxJS - How to replace a Callback in a Subscription with an Observable

Recently a colleague of mine implemented a callback in a Subscription in our Angular 6 application like this:

callDoSomething() {
  this.doSomething(() => { 
    // logic from callback
   });
}

public doSomething(callback: any) {
  this.http.get('/api/doSomething').subscribe(data => {
    // do something with data
    if (callback) {
      callback();
    }
  }, error => {
    // do something with error
  });
}

I thought it would be better to solve this "problem" with an RxJS Observable instead. But after I implemented/refactored the code it doesn't seem to be the better solution. The code looks much more complicated now. Is it necessary to cast the pipe to a promise and return it as an observable?

import { Observable, of as observableOf, from as observableFrom } from 'rxjs';
import { catchError, map } from 'rxjs/operators';

callDoSomething() {
    this.doSomething().subscribe((result) => {
    if (result) {
        // logic from callback
    }
    });
}

public doSomething() {
    let promise = this.http.get('/api/doSomething').pipe(
    map(data => {
        // do something with data
        return observableOf(true);
    }),
    catchError((error) => {
        // do something with error
        return observableOf(false);
    })
    ).toPromise()

    return observableFrom(promise);
}

Is there a better way to solve this problem with Observables? Or was my colleague right to use a simple callback?

Upvotes: 0

Views: 4196

Answers (2)

a better oliver
a better oliver

Reputation: 26828

IMHO the callback is the simplest solution. The complete handler would probably be a better place to call it. If you want to use observables then I would adhere to the stream semantics.

If your goal is to handle the success case and you don't want to get data out of the function then you can return an empty, completed observable. I case of an error you can either let it pass through or replace it with an empty observable, that terminates with an error. It is similar to observable.of(true / false), but relies on the stream semantics instead of arbitrary values. 

    doSomething() {
     return this.http.get('/api/doSomething').pipe(
        flatMap(data => {
          // do something with data
          return empty();
        }),
        catchError((error) => {
          // do something with error
          return throwError(error); //instead of error pass whatever you like
        })
      );
    }

    callDoSomething() {
      this.doSomething()
       .subscribe(
         undefined, //we don't handle data
         undefined, //we don't handle errors
         () => {// logic from callback}
       );
    }

As you can see it's still much more code than in the callback solution. But if you add an error callback one day it might be an interesting option. The upside of the observable solution: No ifs.

Upvotes: 2

Suren Srapyan
Suren Srapyan

Reputation: 68655

You don't need to cast to Promise and then again cast into Observable. Also look at the map function. Why you return true from it ? map function is designed for changing coming data and pass changed ones next.

public doSomething() {
    let observable= this.http.get('/api/doSomething').pipe(
    map(data => {
        // do something with data
        return observableOf(true);
    }),
    catchError((error) => {
        // do something with error
        return observableOf(false);
    }));

    return observable;
}

Upvotes: 1

Related Questions