【リファクタリング】問い合わせと更新の分離を丁寧にやってみた

## はじめに
リファクタリング第 2 版の 11 章の「問い合わせと更新の分離」に
jest のテストを加えながら、一歩ずつリファクタリングを進めていきます.

- Before : 状態を変更しない冪等な「問い合わせ」と状態の変更を行う「更新」が混ざった関数
- After : 状態を変更しない冪等な「問い合わせ」関数と、状態の変更を行う「更新」関数

となるように関数を分けることをゴールとしてます

環境

```bash
v14.14.0 # node
26.6.3 # jest 
```

## リファクタリング前

リファクタリング前は、`alertForMiscreant` は問い合わせと更新が見事に混ざった状態です。

```javascript:index.js
import { setAlerms } from "./alerms";

// TODO : 問い合わせと更新の分離
export function alertForMiscreant(people) {
  for (const p of people) {
    if (p === "Don") {
      setAlerms();
      return "Don";
    }
    if (p === "John") {
      setAlerms();
      return "John";
    }
  }
  return "";
}
```

```javascript:alerms.js
export function setAlerms() {
  // 何かしらの更新処理
}
```

## リファクタリング手順

手順は以下のように進めていきます。

1. テストコードを書いて Green であることをテストする
2. 元の関数をコピーして新しい関数を作成する
3. 新しい関数をから、副作用の部分を取り除いて問い合わせ専用の関数とする
4. 元の関数の問い合わせ部分を、新しい関数で置き換える

### 1. テストコードの追加

`setAlerms` が適切に呼ばれているかをテストするコードを書いていきます。
冗長な初期化処理や、後処理はご容赦ください。

```javascript
import { alertForMiscreant } from "../src/";
import * as alerms from "../src/alerms";

describe("問い合わせと更新の分離", () => {
  test("Don のとき setAlerms が 1 度呼ばれます", () => {
    const spy = jest.spyOn(alerms, "setAlerms").mockImplementation();
    const people = ["Don"];
    alertForMiscreant(people);
    expect(spy).toHaveBeenCalledTimes(1);
    spy.mockRestore();
  });

  test("John, Mar のとき setAlerms が 1 度呼ばれます", () => {
    const spy = jest.spyOn(alerms, "setAlerms").mockImplementation();
    const people = ["John", "Mar"];
    alertForMiscreant(people);
    expect(spy).toHaveBeenCalledTimes(1);
    spy.mockRestore();
  });

  test("Mar, Mar, Pon のとき setAlerms は度呼ばれません", () => {
    const spy = jest.spyOn(alerms, "setAlerms").mockImplementation();
    const people = ["Mar", "Mar", "Pon"];
    alertForMiscreant(people);
    expect(spy).toHaveBeenCalledTimes(0);
    spy.mockRestore();
  });

  ........
});

```

### 2. 元の関数をコピーして新しい関数を作成

ますは、元の関数をまるまるコピーしてきて、名前を変更します。
大げさかなと思うかもしれませんが、テストを繰り返しながら確実に一歩ずつリファクタをすすめる上でこの手順は無視できません。

```javascript:index.js
import { setCombatAlerm } from "./setCombatAlerm";

export function alertForMiscreant(people) {
  for (const p of people) {
    if (p === "Don") {
      setAlerms();
      return "Don";
    }
    if (p === "John") {
      setAlerms();
      return "John";
    }
  }
  return "";
}

// まるまるコピーして、名前だけ変更
export function findMiscreant(people) {
  for (const p of people) {
    if (p === "Don") {
      setAlerms();
      return "Don";
    }
    if (p === "John") {
      setAlerms();
      return "John";
    }
  }
  return "";
}
```

### 3. 新しい関数をから、副作用の部分を取り除いて問い合わせ専用の関数とする

さらにここから、更新に係る処理を削除して、副作用を消します。

```diff:index.js
....
export function findMiscreant(people) {
  for (const p of people) {
    if (p === "Don") {
-     setAlerms();
      return "Don";
    }
    if (p === "John") {
-      setAlerms();
      return "John";
    }
  }
  return "";
}
```

### 4. 元の関数の問い合わせ部分を、新しい関数で置き換える

`alertForMiscreant`  に入っていた問い合わせ部分を、`findMiscreant` で置き換えます。
問い合わせ部分が分離されていることがわかります。

```javascript:index.js
import { setCombatAlerm } from "./setCombatAlerm";

export function alertForMiscreant(people) {
  if (findMiscreant(people) !== '') {
    setAlerms();
}

export function findMiscreant(people) {
  for (const p of people) {
    if (p === "Don") {
      return "Don";
    }
    if (p === "John") {
      return "John";
    }
  }
  return "";
}
```

「問い合わせと更新の分離」の目的は果たせましたが、もう少しだけリファクタを進めておきます。
まだまだやりたい修正はたくさんですが、一旦以下で止めておきます。


```javascript:index.js
import { setCombatAlerm } from "./setCombatAlerm";

export function alertForMiscreant(people) {
  if (someMiscreant(people)) {
    setAlerms();
}

export function someMiscreant(people) {
  return people.some((person) => {
    return ["Don", "John"].includes(person);
  });
}
```

関数が増えるとともに
`alertForMiscreant` の戻り値の型が `string` -> `void`
`someMiscreant` の戻り値の型が `string` -> `boolean`
に変更されましたがテストとステップバイステップの修正のおかげで安全にリファクタをすることができました


## まとめ

正直「問い合わせと更新の分離」は日々自然に行っているなと感じる方も多いのではないでしょうか？
ただ、ここまで確実に一歩ずつやっていたかと言われるともう少し荒いステップだったなと、振り返るきっかけになりました
明日は、もう一歩きれいなコードを書きたいですね


## 参考
- [リファクタリング 既存のコードを安全に改善する（第2版）](https://www.amazon.co.jp/dp/B0827R4BDW)
- [jest spyOn](https://jestjs.io/docs/ja/jest-object#jestspyonobject-methodname)