Node.jsで非同期な処理を直列に実行したくなったが、そうじゃなかった話

こんにちは、いとあきです。
アイスタイルAdvent Calender2019の19日目を担当させていただきます。

18日目のnakanoanさんの記事は、アイスタイルらしさが伝わるとても良い記事でしたね。

私も、さすあきのスタンプを初めて見たときは、とてもうれしい気持ちになりました。
とっても素敵な文化だと思ってます。

さて、私は現在、サーバーサイドエンジニアとして、TypeScript + Node.jsを使用してAPIの開発に携わっています。
その中で、
・基本に立ち返ったつもりでも、実は立ち返れていないのかもしれない
・Node.jsで直列処理を書きたくなったら、今一度メソッドの役割を見直すと解決するかもしれない
ということを学んだので、「Node.jsで直列処理をしたくなったのだが、そうじゃなかった話」と題して投稿させていただきます。

きっかけ

直列処理を実装したくなったきっかけは、複数件の画像を削除する実装をしているときに訪れました。
もともと画像を削除するcdn.deleteというクラスが存在していたので、そいつを使って下記のようなコードを書きました。
※本投稿のコードは、tech blog用に書き直したコードになっております

    await Promise.all([
      ...images.map(image => this.cdn.delete(image.path))
    ]);

しかし、この実装では、正常に処理が終了しないパターンがありました。

cdn.deleteの中では、
1.指定されたパスの画像を削除する
2.ディレクトリの中身に画像がなくなったらディレクトリも削除する
という処理が実装されています。

この改修が入るまでは、1件の画像を削除する用途でしか使われていなかったので、cdn.deleteは正しく動作していました。
ですが、上記の実装では、非同期に処理が走るため、複数の処理が2まで到達してしまい、No such fileエラーが発生してしまいます。

覚えたことを駆使して頑張ってみた

「直列に処理が実行されれば、最後の1件だけディレクトリの削除処理が実行されるはず。」
解決策を検討していく中で、Array.prototype.reduce()を使って、非同期な処理を直列に実行する方法にたどり着きました。

    images.reduce((promise, image) => (
      promise.then(() => this.cdn.delete(image.path))
    ), Promise.resolve());

これで、正常に動作する、且つ今まで覚えたreduceやPromise.thenを駆使したいいコードが書けたなと満足げな表情を浮かべながらコミットコマンドを打つのでした。

そうじゃなかった

「for文のほうが見やすいのでは?」
そんな指摘を受けました。

    for (const image of images) {
      await this.cdn.delete(image.path);
    }

実は、私がreduceやPromise.thenを駆使して頑張って書いたコードでやっていたことは、for文でも実現できたのです。
こちらのほうがパッと見て何をしているかわかるので、後で他の人が見たときにもわかりやすいですよね。

色々覚えたことは使いたくなってしまうものですが、ですが、他の人が見たときにぱっとみて何をしているかが分かるコードを書いたほうがみんなが幸せになれる未来が待っている。そんな基本的なことを改めて学ぶ出来事でした。

これでコードも見やすくなったし、エラーも発生しなくなって万事解決です。

そうじゃなかった(かもしれない)

いいコード書いたなー、学んだなーとすがすがしい気持ちになっていたのですが、次の日の朝、ふと頭をよぎります。
「そもそもimageの削除とディレクトリの削除が一緒になっているのが良くないのでは?」

cdn.deleteの中では、
1.指定されたパスの画像を削除する
のみを実行し、非同期で画像を削除してから、ディレクトリを削除すればそもそも直列処理を入れる必要もなさそうということに気が付きました。

    // cdn.deleteの中では、画像の削除のみ行い、ディレクトリの削除は行わない
    await Promise.all([
      ...images.map(image => this.cdn.delete(image.path))
    ]);

    // 新しくディレクトリ削除のメソッドを作成し、画像を非同期で削除した後にディレクトリを削除すればよいのでは?
    await this.cdn.deleteDir(path);

もともと、1件のみの画像削除用のメソッドとして使われていた、cdn.deleteですが、複数件の画像を削除する用途が出てきたときに、メソッドのあるべき姿をもう一度見直すべきだったと学びました。
初心に立ち返ったつもりでfor文に直したのですが、もっと外側からの視点で改修を検討すべきでした。

おわりに

「標的が獲物を狩る瞬間に隙はできる」
人間これだ!と思った瞬間には、視野が狭くなってしまっている可能性があります。
そんな時こそ、今一度周りを見渡してみると、よりよい解決策が転がっているかもしれません。

アイスタイルでは一緒によりよいコード、より良いプロダクト開発を目指す仲間を募集しています。面白そう、もっと知りたいと思われた方はぜひオフィスに遊びにお越し下さい!ご応募はこちらからお願いします。

最後までお読みいただきありがとうございました。

楽しく成長できる職場を目指す、サーバーサイドエンジニアです。

コメントを残す