diff --git a/src/pages/drafts/tips/beautiful-coding.mdx b/src/pages/drafts/tips/beautiful-coding.mdx index 4c9eeae..2033d9e 100644 --- a/src/pages/drafts/tips/beautiful-coding.mdx +++ b/src/pages/drafts/tips/beautiful-coding.mdx @@ -2,36 +2,29 @@ layout: "@/layouts/MarkdownLayout.astro" --- +import Details from "@/components/Details.astro"; +import Dialog from "@/components/Dialog.astro"; + export const title = "きれいなコードってなに?"; # {title} -パット見で、なにをするソースコードかわかりますか? -3秒以内に答えてください。 +## TOC ---- +## 関数の役割 -
-ソースコードを見る +パッと見で、なにをするソースコードかわかりますか? +3秒以内に答えてみてください。 + + ```js function main() { - const data = [ - { name: "もつに", scores: { math: 124, english: 34 } }, - { name: "なおき", scores: { math: 56, english: 78 } }, - { name: "たかし", scores: { math: 90, english: 12 } }, - // ... - ]; + 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].total === undefined) { - data[i].total = data[i].scores.math + data[i].scores.english; - } - if (data[j].total === undefined) { - data[j].total = data[j].scores.math + data[j].scores.english; - } - if (data[i].total > data[j].total) { + if (data[i] > data[j]) { const temp = data[i]; data[i] = data[j]; data[j] = temp; @@ -45,41 +38,121 @@ main(); ``` -
+ + +
+
+ +答えられましたか? +わかった人もわからなかった人も、このソースコードではどうでしょうか? + + + +```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(); +``` + + + +
+
+ +
+ +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関数の中にあったループ処理が別の関数として切り出されていることがわかります。 --- -誰かがこっちのほうが早いと言って、以下のように修正しました。 -出力結果は同じです。 +ところで、最初の質問はこのようなものでした。 -さてもう1度質問します。なんのソースコードだったでしょうか? +> パッと見で、なにをするソースコードかわかりますか? +> 3秒以内に答えてみてください。 ---- +**関数に切り出されたループ部分が、ソート処理である** というところまで理解できる人はほぼほぼいないでしょう。 +しかし、2つ目のソースコードで、その部分を`sort`関数として切り出しました。 +これにより、ループの中身を詳しく理解せずとも、**この関数はデータをソートするんだな** と瞬時に理解できるようになったと思います。 -
-ソースコードを見る +つまり関数の役割とは単に処理を列挙することではなく、**その関数がなにをするのかを表現すること** でもあるのです。 + +
+ +## いい関数、悪い関数 + +以下のソースコードを考えてみましょう。 +これは[関数の役割](#関数の役割)で使ったソースコードを少し改変したものです。 + +
```js function main() { const data = [ - { name: "もつに", scores: { math: 124, english: 34 } }, - { name: "なおき", scores: { math: 56, english: 78 } }, - { name: "たかし", scores: { math: 90, english: 12 } }, + { name: "出来杉英才", score: 100 }, + { name: "野比のび太", score: 0 }, + { name: "剛田武", score: 32 }, // ... ]; - for (let i = 0; i < data.length - 1; i++) { - for (let j = 0; j < data.length - i - 1; j++) { - if (data[j].total === undefined) { - data[j].total = data[j].scores.math + data[j].scores.english; - } - if (data[j + 1].total === undefined) { - data[j + 1].total = data[j + 1].scores.math + data[j + 1].scores.english; - } - if (data[j].total > data[j + 1].total) { - const temp = data[j]; - data[j] = data[j + 1]; - data[j + 1] = temp; + 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; } } } @@ -90,35 +163,75 @@ main(); ``` -
+
+ +
+
+ +こんな依頼がきたとします。 + +> 30点以下は赤点なので、該当者の名前の前に`(赤点)`をつけてほしい! + +今のソースコードに、この**条件を追加することだけを考える**と、以下のように書けます。 + +なお、これは悪い例です。 +なにが良くないのか、考えながら見てみましょう。 + +
+ +```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(); +``` + +

-これかな?と想像まではついたかもしれませんが、確信は持てないのではないでしょうか? +なにが良くないのか思いつきましたか? ---- +ところで今の条件式を追加する前にさかのぼり、予めループ処理を関数に切り出していたとします。 +ここには赤点のときの条件は含まれていません。 -では、これではわかるのではないでしょうか? +ここに赤点の条件を追加するとなると、あなたならどのようなソースコードを書くでしょうか? ---- - -
-ソースコードを見る +
```js -function sort(data) { - for (let i = 0; i < data.length - 1; i++) { - for (let j = 0; j < data.length - i - 1; j++) { - if (data[j].total === undefined) { - data[j].total = data[j].scores.math + data[j].scores.english; - } - if (data[j + 1].total === undefined) { - data[j + 1].total = data[j + 1].scores.math + data[j + 1].scores.english; - } - if (data[j].total > data[j + 1].total) { - const temp = data[j]; - data[j] = data[j + 1]; - data[j + 1] = temp; +/* + * 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; } } } @@ -126,13 +239,13 @@ function main() { const data = [ - { name: "もつに", scores: { math: 124, english: 34 } }, - { name: "なおき", scores: { math: 56, english: 78 } }, - { name: "たかし", scores: { math: 90, english: 12 } }, + { name: "出来杉英才", score: 100 }, + { name: "野比のび太", score: 0 }, + { name: "剛田武", score: 32 }, // ... ]; - sort(data); + sortUserByScore(data); console.log(data); } @@ -140,51 +253,26 @@ main(); ``` -
+
-
-出力結果を見る +
+
-```plain -[ - { name: 'たかし', scores: { math: 90, english: 12 }, total: 102 }, - { name: 'なおき', scores: { math: 56, english: 78 }, total: 134 }, - { name: 'もつに', scores: { math: 124, english: 34 }, total: 158 } -] -``` +前述の悪いコードの書き方だと、切り出した関数の中に赤点の条件を追加していることになりますね。 -
+
-
-答えを見る - -これは、テストの点数の合計で生徒を昇順に並び替えるソースコードです。 - -
- -ことろで、こんな不具合報告が来たとします。 - -> 合計がおかしいです! - -さて、今のソースコードを見て、どこを直せばよいかわかりますか? - -
-ソースコードを見る - -```js -function sort(data) { - for (let i = 0; i < data.length - 1; i++) { - for (let j = 0; j < data.length - i - 1; j++) { - if (data[j].total === undefined) { - data[j].total = data[j].scores.math + data[j].scores.english; +```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[j + 1].total === undefined) { - data[j + 1].total = data[j + 1].scores.math + data[j + 1].scores.english; - } - if (data[j].total > data[j + 1].total) { - const temp = data[j]; - data[j] = data[j + 1]; - data[j + 1] = temp; + if (data[i].score > data[j].score) { + const temp = data[i]; + data[i] = data[j]; + data[j] = temp; } } } @@ -192,13 +280,13 @@ function main() { const data = [ - { name: "もつに", scores: { math: 124, english: 34 } }, - { name: "なおき", scores: { math: 56, english: 78 } }, - { name: "たかし", scores: { math: 90, english: 12 } }, + { name: "出来杉英才", score: 100 }, + { name: "野比のび太", score: 0 }, + { name: "剛田武", score: 32 }, // ... ]; - sort(data); + sortUserByScore(data); console.log(data); } @@ -206,4 +294,130 @@ main(); ``` -
+
+ +
+ +この関数は名前にもなっている`sortUserByScore` (ユーザーをスコアでソートする) という役割とは別に、赤点の人の名前に`(赤点)`をつけるという役割も持つことになってしまいました。 +これが良くないのです。 + +より客観的に状況を整理すると、同じ範囲の中で**ソートをする処理**と**赤点の条件をつける処理**の2つの役割が混在してしまっている状態です。 + +
+ +```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; + } + } +} +``` + +
+ +
+ +依頼というものは突拍子もなくやってきます。 +このコードを書いて数日経って修正することになるとき、あなたはマークの条件をループの中に書いたことを**思い出すことができるでしょうか?** + +また、もし関数を切り出していたとき、呼び出し元のmain関数からは、`sortUserByScore`関数がソートをすることしかわかりません。 +それはmain関数からしたら、いつの間にか値が変わっていたという状況に他なりません。 + +大きくなっていくソースコードでこの条件1つを探すために、main関数全体をくまなく探すことになってもいいでしょうか? + +これを防ぐために、処理は**1つの役割だけを持つべき** という原則があります。 +この1つだけの役割を組み合わせることで、複雑な処理を実現していくことを強く推奨します。 + +
+
+ +特別な理由がなければ以下のように、きちんと役割で分割しましょう。 + +
+ +```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(); +``` + +
+ +
+
+ +これが徹底されていれば関数に切り出しても必要以上に確認する範囲が増えず、修正も楽になります。 + +
+ +```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(); +``` + +