HttpClient で受け取ったレスポンスをサービス内で処理するのは不適切ですか？

[lacolaco]

Posted by honda28 on 2021-09-02T01:56:31.948Z

Angular 歴1ヶ月ほどの初心者です。現在他者が作成したアプリケーションのコードを読んでおり、表記の疑問が出たので質問させてください。

【質問】

サービス内でデータの Http リクエストを送って返ってきたレスポンスの処理まで行うのは不適切ですか？
（サービスはレスポンスを返すまでとし、レスポンスの処理はコンポーネント内で行うべきですか？）

【詳細】

私が読んでいるコードでは、コード例の api.service.ts のようにサービス内では、Observable レスポンスを渡すまでとなっており、app.component.ts にて subscribe して処理が行われています。

そのようなコードだと、同じサービスを使用するコンポーネントすべてで処理を書く必要があり、冗長に感じました。例えば、上記リンクのコードを以下のように修正すればコンポーネント側ではサービスのメソッドを使用するだけでよく、シンプルなコードになると思いました。

[api.service.ts]

import { Injectable } from '@angular/core';
import { HttpClient } from '@angular/common/http';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
const API_URL = 'https://reqres.in';

@Injectable({

providedIn: 'root'

})

export class ApiService {

users: any; // プロパティ追加
constructor(private http: HttpClient) { }
// subscribe 処理まで記述

public get(url) {

return this.http.get(API_URL + '/api/' + url).subscribe(res => {

this.users = res;

console.log('data response', this.users);

});

}
// このメソッドを使用すれば users データを取得できる

public set() {

return this.users;

}

}

[app.component.ts]

import { Component, OnInit } from '@angular/core';
import { ApiService } from './api.service';
@Component({

selector: 'my-app',

templateUrl: './app.component.html',

styleUrls: [ './app.component.css' ]

})

export class AppComponent implements OnInit {

users: any;
constructor(private api: ApiService) { }
ngOnInit() {

this.api.get('users?page=1');

// サービスの set メソッドを用いて users にデータをセットする

this.users = this.api.set();

}

}

このような書き方は不適切でしょうか？また「①サービスがレスポンスを返すまでとする」、「②サービスがレスポンスの処理まで行う」のどちらが適切かに関わらず、それぞれのメリット・デメリットもわかる方がいればご教授いただけると幸いです。

[lacolaco]

Posted by rdlabo on 2021-09-02T03:05:21.998Z

サンプルコードはエンドポイントの同一レスポンスを使いまわしたい時にはよくします。これをすると、2回目からのリクエストはエンドポイントにアクセスしなくていいので、高速化、またサーバへの負荷軽減が見込めるのでいいですよね。

質問の「Serviceでどこまで処理をするか」についてですが、私は

・共通化できない部分があればそれはComponentsで処理する
・それ以外はすべてServiceで行う

というルールで処理しています。Serviceは細かくファイルを句切ればあまりFatになりませんが、Componentsは複雑な処理をするとすぐに行数が膨れ上がってるので、Serviceにおいた方が見通しがいい構成になることが多いです。

[lacolaco]

Posted by lacolaco on 2021-09-02T03:43:30.606Z

ある処理をサービスでやるかコンポーネントでやるかについては、その処理がユースケースに依存するかどうかで考えることが多いです。

たとえばバックエンドから返ってくるUserのJSONがフロントエンドで扱いにくいために整形したオブジェクトにする、ということならそれはフロントエンドの中でそのデータを「何に使うか」に依存しないので、サービスに含めています。
一方で、たとえばユーザー一覧とユーザー詳細それぞれのためにデータを整形するとなると、それはそれぞれのユースケースに依存するので（少なくとも『APIを呼び出して結果を受け取る』という責務の）サービスではやりません。

結局そのサービスの責務の切り方は？ということになるので、ApiServiceなら ApiService という名前で過不足ない範囲の仕事に収めることが重要かと思います。

[lacolaco]

Posted by honda28 on 2021-09-02T04:20:33.375Z

rdlabo さん、迅速で丁寧なご回答ありがとうございます。

質問の時点では「コードのわかりやすさやメンテナンス性」という観点しかなかったので、「高速化やサーバの負荷軽減」というメリットには気づいていませんでした。ご教授いただき、ありがとうございます。

私が現在読んでいるコードでは、用途に合わせて API 自体が細かく分けられています。ですので、今回のコードに関しては共通化できそうなので、自身が始めに思ったようにサービス内で処理までを行おうかと思います。
ありがとうございました。

[lacolaco]

Posted by akai on 2021-09-02T04:37:22.859Z

主題からは離れてしまうのですが、本コードはおそらくバギーなコードになっています。

  ngOnInit() {
    this.api.get('users?page=1');
    // サービスの set メソッドを用いて users にデータをセットする
    this.users = this.api.set();
  }

this.usersに直前のgetで取得されるuser郡が、setで入ることを想定されているかと思われますが、
get関数の処理は非同期で行われるため、
「サービスで管理されている古い値、もしくは初期値」がset関数の戻り値として入ってきます。

提示されたコードのように処理を分けるのであれば、
toPromise(rxjs7だったらlastFromValueかな？)などを用いて、
非同期処理を同期的に解決できるようなコードを書く必要があるはずです。

サンプルコードでは、
こういった本筋とはずれる非同期周りの取り回しをコードに入れて複雑にしたくなかったために
こういう書き方になっているのかもしれませんね。

---

主題の HttpClient で受け取ったレスポンスをサービス内で処理する ことについては、
概ねはサービスにロジックを寄せるべきであることが多いと思いますので、よいと思います。

[lacolaco]

Posted by honda28 on 2021-09-02T04:42:15.284Z

lacolaco さん、迅速で丁寧なご回答ありがとうございます。

一般にどちらでやるものではなく、ユースケースを考慮するということですね。
認識が合っているか不安なので、以下認識が正しいか確認させてください。

【認識確認】

上記で挙げられた（ユーザ一覧とユーザ詳細に関する）例は、バックエンドから以下のような配列が返ってくるような API を想定した話であると認識しています。

[
  user1: { age: 20, hobby: soccer },
  user2: { age: 55, hobby: traveling },
  ...
]

このような配列が返ってきた際に「ユーザ一覧」と「ユーザ詳細」の両方を扱いたい場合は、どちらかを扱うかによって処理が異なる（ユースケースに依存する）ため、1つのサービスでは処理すべきでない、というお話と認識しています。

この場合、「ユーザ一覧用サービス UserListService」 と 「ユーザ詳細用サービス UserDetailsService」 というようにサービス自体をそれぞれ分離して、その上で処理をサービスに内包するのであれば（名前の範囲内の仕事であるので）問題ない、という認識です。正しく lacolaco さんの話を理解できていますでしょうか？

[lacolaco]

Posted by honda28 on 2021-09-02T04:49:16.732Z

akai さん、ご回答ありがとうございます。

たしかに仰るとおりですね、ご指摘ありがとうございます。本筋に関わらないため意図的にこのようにしたわけではなく、単純に気づいていませんでした。今後同様の処理を行う際にハマる可能性もあるので、事前に教えていただけてとても助かりました。

下記のように、async / await で非同期処理が実現できるかなと思いました。

  async ngOnInit() {
    await this.api.get('users?page=1');
    // サービスの set メソッドを用いて users にデータをセットする
    this.users = this.api.set();
  }

[lacolaco]

Posted by akai on 2021-09-02T05:20:26.272Z

api.service.tsのget関数が返す値はPromiseではないので、そのままではasync/awaitは利用できません。
api.service.tsも書き換える必要があります。
(おそらく、ご理解されているとは思いますが、念の為……。)

---

ちなみに、よくあるのは以下のようなイメージですかね。
(提示されたコードのように、usersをキャッシュするのであれば、ApiServiceではなくUserServiceなどの名前にした方がよいでしょうね。 次のコードではusersのキャッシュは行っていません。)

[api.service.ts]

/// 省略
export class ApiService {
  constructor(private http: HttpClient) { }
public get(url: string) {

return this.http.get(API_URL + '/api/' + url).toPromise();

}

}

[app.component.ts]

/// 省略
export class AppComponent implements OnInit {
  users: User[];
constructor(private api: ApiService) {}
async ngOnInit() {

this.users = await this.api.get('users?page=1');

}

}

[lacolaco]

Posted by honda28 on 2021-09-02T05:49:30.457Z

akai さん、ご丁寧にありがとうございます。

仰るとおり、Observable を toPromise で Promise に変換する必要がありますね。Angular 同様、JavaScript (TypeScript) の経験も浅いので、細かくご指摘いただけて非常に助かります。

サービス内で処理を行うのであれば、さらに then / catch を追記するイメージですかね。

[lacolaco]

Posted by akai on 2021-09-02T06:01:01.468Z

サービス内で処理を行うのであれば、さらに then / catch を追記するイメージですかね。

そのようになります。 また、then/catchを利用せずに、RxJSを利用して複雑な実装を行うことも可能です。

public get(url: string) {
  const source = this.http.get(API_URL + '/api/' + url).pipe(
    debounceTime(100), // ボタン連打で無意味な通信が走る可能性を減らすとか
    map(res => ({ ...res, date: new Date() })), // mapで値書き換えるとか
    tap(res => console.log('log:', res)), //処理とは関係ない作用を挟むとか
    finalize(res => alert('通信が終わったよ')) // エラーでもエラーじゃなくても最後にアラート呼び出したりとか
  );
  return source.toPromise();
}

[lacolaco]

Posted by lacolaco on 2021-09-02T06:07:46.531Z

この場合、 「ユーザ一覧用サービス UserListService」 と 「ユーザ詳細用サービス UserDetailsService」 というようにサービス自体をそれぞれ分離して、その上で処理をサービスに内包するのであれば（名前の範囲内の仕事であるので）問題ない、という認識です。

はい、それならサービスの名前と責務が一致していてメンテナンスしやすいのではないかと思います。 さらに、UserListService と UserDetailsService に共通するHTTP通信の部分は UserApiService などに分けていけるとそれぞれ小さくテストしやすいモジュールになっていくと思います！

[lacolaco]

Posted by honda28 on 2021-09-02T06:12:11.490Z

akai さん、丁寧なご回答ありがとうございます。

RxJS を利用するとそんなに便利なこともできるんですね、とても勉強になりました！コードとしても非常に見やすいですし、ぜひ参考にさせていただこうと思います！

[lacolaco]

Posted by honda28 on 2021-09-02T06:31:57.185Z

lacolaco さん、丁寧なご回答ありがとうございます。

認識が合っていたようで安心しました。またサービスの共通部分をさらにサービスとして分ける（UserApiService を UserListService と UserDetailsService のそれぞれに注入するという認識です）というのも発想がなかったので、教えていただけて非常にためになりました。

こちらのコミュニティでの初の質問でしたが、多くの方から丁寧かつ勉強になるコメントをたくさんいただくことができ、非常に助かりました。lacolaco さんはコミュニティ管理者の方とのことですので、ぜひとも引き続きこの良いコミュニティの管理を続けていただければと思います。私も回答者側になれるように勉強したいと思います。本当にありがとうございました。