Newer
Older
hello-programmer-world / src / pages / tips / beautiful-coding.mdx
---
layout: "@/layouts/MarkdownLayout.astro"
---

import Details from "@/components/Details.astro";
import Dialog from "@/components/Dialog.astro";

export const title = "きれいなコードってなに?";

# {title}

## TOC

## 関数の役割

パッと見で、なにをするソースコードかわかりますか?  
3秒以内に答えてみてください。

<Dialog title="ソースコードを見る">

```js
function main() {
  const data = [124, 34, 56, 78, 90, 12];

  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i] > data[j]) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }

  console.log(data);
}

main();
```

</Dialog>

<br />
<br />

答えられましたか?  
わかった人もわからなかった人も、このソースコードではどうでしょうか?

<Dialog title="ソースコードを見る">

```js
function sort(data) {
  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i] > data[j]) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
}

function main() {
  const data = [124, 34, 56, 78, 90, 12];

  sort(data);

  console.log(data);
}

main();
```

</Dialog>

<br />
<br />

<Details summary="解説">

2つのソースコードを比較して、どこが変わっていたでしょうか?

```js showLineNumbers=false add={1-11,25} del={16-24}
function sort(data) {
  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i] > data[j]) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
}

function main() {
  const data = [124, 34, 56, 78, 90, 12];

  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i] > data[j]) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
  sort(data);

  console.log(data);
}

main();
```

比較してみると、main関数の中にあったループ処理が別の関数として切り出されていることがわかります。

---

ところで、最初の質問はこのようなものでした。

> パッと見で、なにをするソースコードかわかりますか?  
> 3秒以内に答えてみてください。

**関数に切り出されたループ部分が、ソート処理である** というところまで理解できる人はあまりいないでしょう。  
しかし、2つ目のソースコードで、その部分を`sort`関数として切り出しました。  
これにより、ループの中身を詳しく理解せずとも、**この関数はデータをソートするんだな** と瞬時に理解できるようになったと思います。

つまり関数の役割とは単に処理を列挙することではなく、**その関数がなにをするのかを表現すること** でもあるのです。

</Details>

## いい関数、悪い関数

以下のソースコードを考えてみましょう。  
これは[関数の役割](#関数の役割)で使ったソースコードを少し改変したものです。

<Details summary="ソースコードを見る">

```js
function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }

  console.log(data);
}

main();
```

</Details>

<br />
<br />

こんな依頼がきたとします。

> 30点以下は赤点なので、該当者の名前の前に`(赤点)`をつけてほしい!

今のソースコードに、この**条件を追加することだけを考える**と、以下のように書けます。

なお、これは悪い例です。  
なにが良くないのか、考えながら見てみましょう。

<Details summary="ソースコードを見る">

```js add={10-12}
function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  for (let i = 0; i < data.length; i++) {
    if (data[i].score <= 30) {
      data[i].name = "(赤点) " + data[i].name;
    }
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }

  console.log(data);
}

main();
```

</Details>

<br />

なにが良くないのか思いつきましたか?

ところで今の条件式を追加する前にさかのぼり、予めループ処理を関数に切り出していたとします。  
ここには赤点のときの条件は含まれていません。

ここに赤点の条件を追加するとなると、あなたならどのようなソースコードを書くでしょうか?

<Details summary="ソースコードを見る">

```js
/*
 * TODO: どこかに赤点の条件を追加したい
 */

function sortUserByScore(data) {
  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
}

function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  sortUserByScore(data);

  console.log(data);
}

main();
```

</Details>

<br />
<br />

前述の悪いコードの書き方だと、切り出した関数の中に赤点の条件を追加していることになりますね。

<Details summary="ソースコードを見る">

```js add={4-6}
function sortUserByScore(data) {
  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score <= 30) {
        data[i].name = "(赤点) " + data[i].name;
      }
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
}

function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  sortUserByScore(data);

  console.log(data);
}

main();
```

</Details>

<br />

この関数は名前にもなっている`sortUserByScore` (ユーザーをスコアでソートする) という役割とは別に、赤点の人の名前に`(赤点)`をつけるという役割も持つことになってしまいました。  
これが良くないのです。

より客観的に状況を整理すると、同じ範囲の中で**ソートをする処理**と**赤点の条件をつける処理**の2つの役割が混在してしまっている状態です。

<Details summary="ソースコードを見る">

```js {"1. ソートをするための処理":1-14} ins={"2. 30点以下なら赤点とマーク":4-7} showLineNumbers=false
// ソート
for (let i = 0; i < data.length; i++) {
  for (let j = i + 1; j < data.length; j++) {
    // マーク
    if (data[i].score <= 30) {
      data[i].name = "(赤点) " + data[i].name;
    }
    if (data[i].score > data[j].score) {
      const temp = data[i];
      data[i] = data[j];
      data[j] = temp;
    }
  }
}
```

</Details>

<br />

依頼というものは突拍子もなくやってきます。  
このコードを書いて数日経って修正することになるとき、あなたはマークの条件をループの中に書いたことを**思い出すことができるでしょうか?**

また、もし関数を切り出していたとき、呼び出し元のmain関数からは、`sortUserByScore`関数がソートをすることしかわかりません。  
それはmain関数からしたら、いつの間にか値が変わっていたという状況に他なりません。

大きくなっていくソースコードでこの条件1つを探すために、main関数全体をくまなく探すことになってもいいでしょうか?

これを防ぐために、処理は**1つの役割だけを持つべき** という原則があります。  
この1つだけの役割を組み合わせることで、複雑な処理を実現していくことを強く推奨します。

<br />
<br />

特別な理由がなければ以下のように、きちんと役割で分割しましょう。

<Details summary="ソースコードを見る">

```js {"1. ソートをするための処理":8-17} {"2. 30点以下なら赤点とマーク":19-24}
function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }


  for (let i = 0; i < data.length; i++) {
    if (data[i].score <= 30) {
      data[i].name = "(赤点) " + data[i].name;
    }
  }

  console.log(data);
}

main();
```

</Details>

<br />
<br />

これが徹底されていれば関数に切り出しても必要以上に確認する範囲が増えず、修正も楽になります。  
~~やりすぎも良くないですが、~~ ぜひ心がけてみてください。

<Details summary="ソースコードを見る">

```js
function sortUserByScore(data) {
  for (let i = 0; i < data.length; i++) {
    for (let j = i + 1; j < data.length; j++) {
      if (data[i].score > data[j].score) {
        const temp = data[i];
        data[i] = data[j];
        data[j] = temp;
      }
    }
  }
}

function main() {
  const data = [
    { name: "出来杉英才", score: 100 },
    { name: "野比のび太", score: 0 },
    { name: "剛田武", score: 32 },
    // ...
  ];

  sortUserByScore(data);

  // さほど大事な処理ではないので関数化はしない
  for (let i = 0; i < data.length; i++) {
    if (data[i].score <= 30) {
      data[i].name = "(赤点) " + data[i].name;
    }
  }

  console.log(data);
}

main();
```

</Details>