Let's say I have a component like this

The Original Code:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  fetchUsers() {
    this._http.get(`https://jsonplaceholder.typicode.com/users`).subscribe(
      (resp) => {
        console.log(resp);
        this.response = resp;
      },
      (err) => {
        console.error(err);
        this.response = err;
      },
      () => {
        console.log('Subscription Complete');
      }
    );
  }
}

and I have following approaches to refactor the above code...

Refactor approach 1:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  onSubscribe = (resp: any) => {
    console.log(resp);
    this.response = resp;
  };

  one rror = (err: any) => {
    console.error(err);
    this.response = err;
  };

  onCompletion = () => {
    console.log('Subscription Complete');
  };

  fetchUsers() {
    this._http
      .get(`https://jsonplaceholder.typicode.com/users`)
      .subscribe(this.onSubscribe, this.onError, this.onCompletion);
  }
}

Refactor approach 2:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  onSubscribe(resp: any) {
    console.log(resp);
    this.response = resp;
  }

  one rror(err: any) {
    console.error(err);
    this.response = err;
  }

  onCompletion() {
    console.log('Subscription Complete');
  }

  fetchUsers() {
    this._http.get(`https://jsonplaceholder.typicode.com/users`).subscribe(
      (resp) => this.onSubscribe(resp),
      (err) => this.onError(err),
      () => this.onCompletion()
    );
  }
}

Refactor approach 3:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  fetchUsers() {
    this._http.get(`https://jsonplaceholder.typicode.com/users`).subscribe({
      next: (resp: any) => {
        console.log(resp);
        this.response = resp;
      },
      error: (err: any) => {
        console.error(err);
        this.response = err;
      },
      complete: () => {
        console.log('Subscription Complete');
      },
    });
  }
}

Refactor approach 4:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  onSubscribe = (resp: any) => {
    console.log(resp);
    this.response = resp;
  };

  one rror = (err: any) => {
    console.error(err);
    this.response = err;
  };

  onCompletion = () => {
    console.log('Subscription Complete');
  };

  fetchUsers() {
    this._http.get(`https://jsonplaceholder.typicode.com/users`).subscribe({
      next: this.onSubscribe,
      error: this.onError,
      complete: this.onCompletion,
    });
  }
}

Refactor approach 5:

import { HttpClient } from '@angular/common/http';
import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<pre>{{ response | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  constructor(private _http: HttpClient) {
    this.fetchUsers();
  }

  response!: any;

  onSubscribe(resp: any) {
    console.log(resp);
    this.response = resp;
  }

  one rror(err: any) {
    console.error(err);
    this.response = err;
  }

  onCompletion() {
    console.log('Subscription Complete');
  }

  fetchUsers() {
    this._http.get(`https://jsonplaceholder.typicode.com/users`).subscribe({
      next: (resp: any) => this.onSubscribe(resp),
      error: (err: any) => this.onError(err),
      complete: () => this.onCompletion(),
    });
  }
}

Now the question is

Considering the performance (in the first place & readability the next) - which would be the best choice?

1. Refactor approach 1?
2. Refactor approach 2?
3. Refactor approach 3?
4. Refactor approach 4?
5. Refactor approach 5?
6. The Original Code?

CodePudding user response：

I think that the best approach is the number 4 for the next reasons:

Talking about readability, the way in that you pass three arguments on the subscription (next, error, complete) is actually deprecated for RxJS because function arguments can contribute to hard-to-read-code. You can read more about it here

instead, RxJS advises you to use a JS object like an argument where you define the code to the different callbacks

({next: () =>{}, error: () =>{}, complete: () =>{})

And separating the code into functions can help you make it cleaner to read.

Talking about the performance if you pass an argument like a JS object to the subscription RxJS passes an empty function to one of the callbacks instead of you doing it on your own.

CodePudding user response：

Personally, I'd refactor to a declarative approach

1. Create a service to encapsulate all of the data access.
2. In the service, define a variable (not a method) to manage the Observable returned from the http get.
3. In the component, define a variable (not the constructor or life cycle hook) to manage the Observable returned from the service
4. Use the async pipe in the template.

Example Service:

import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { tap } from 'rxjs';

@Injectable({
  providedIn: 'root',
})
export class UserService {
  users$ = this.http
    .get(`https://jsonplaceholder.typicode.com/users`)
    .pipe(tap((response) => console.log(response)));

  constructor(private http: HttpClient) {}

  fetchUsers() {}
}

Example Component/template

import { Component } from '@angular/core';
import { catchError } from 'rxjs';
import { UserService } from './user.service';

@Component({
  selector: 'my-app',
  template: `<pre>{{ users$ | async | json }}</pre>`,
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  errorMessage = '';

  users$ = this.userService.users$.pipe(
    catchError(err => this.errorMessage = err)
  )

  constructor(private userService: UserService) { }

}

See this for more information on this declarative pattern: https://youtu.be/0XPxUa8u-LY