Angular 2 observable subscribe arrow functions getting big and dirty

Question

I want to know if there is a better way to define callback functions of angular 2 observable subscribe when dealing with http calls without violating Single responsibility principle when it comes to embedded logic witch leads to an ugly dirty code.

I am trying to use function variables instead of arrow functions to separate callbacks logic but I can't access this and local function variables (state in the example).

updateState(state: string) {
  let proposition = new Proposition();
  proposition.id = this.id;
  proposition.state = state;
  this.propositionService.updateProposition(proposition).subscribe(
    (data) => {
    ....
    // instruction using local variable
    this.router.navigate(['/portfolio', state]);
    ....
  },
    .....
    // instrution using this
    (errors) => this.toastr.warning('Error.', 'ops !');
    .....}

Answer 1

There are many options and all have upsides and downsides. You should choose the one with the most upsides and the fewest downsides on a case by case basis.

Here are a few options (there are many more)

1. Create a local binding for an arrow function.

   updateState(state: string) {
     const withNext = (data: { values: {}[] }) => {
       console.info(data.values);
       ....
       // instruction using local variable
       this.router.navigate(['/portfolio', state]);
       ....
     };
   
     const withError = error => {
       this.toastr.warning('Error.', error);
     }
   
     this.propositionService.updateProposition(proposition)
       .subscribe(withNext, withError);
   }
   

   The downsides of this approach are that you need to create the callbacks before you use them, because the assignments will not be hoisted, and that you the lose type inference of the callback arguments, needing to restate the argument types redundantly.

2. To get around the declaration order issue, we can create a local function declaration

   updateState(state: string) {
     this.propositionService.updateProposition(proposition)
       .subscribe(withNext, withError);
   
     const that = this;
   
     function withNext(data: { values: {}[] }) {
       console.info(data.values);
       ....
       // instruction using local variable
       that.router.navigate(['/portfolio', state]);
       ....
     }
   
     function withError(error) {
       that.toastr.warning('Error.', error);
     }
   }
   

   The downsides of this approach are that you need to alias this, and that again, the we lose type inference and must resort to redundantly and perhaps incorrectly manually specifying the argument types of the callbacks.

3. If the observable only emits a single value, for example if it represents an HTTP request, we can use toPromise and enjoy clear and clean code with full type inference and no need for callbacks.

   async updateState(state: string) {
     try {
       const data = await this.propositionService.updateProposition(proposition)
         .toPromise();
       console.info(data.values);
       ....
       // instruction using local variable
       this.router.navigate(['/portfolio', state]);
       ....
     } catch (error) {
       this.toastr.warning('Error.', error);
     }
   }
   

   The downside is that this approach only works for observables that emit at most a single value (e.g. HTTP requests).

The state parameter is accessible to all local declarations regardless of approach and is not a factor unless you wish to extract the success and failure logic to a location outside of the updateState method.