Panda Noir

JavaScript の限界を究めるブログです。

スパゲッティコードを書かないために

スパゲッティコードは百害あって一利なし、です。今の自分しか読めません。コーディングスタイルを変えていきましょう。

実際に僕が作ってしまった関数を参考にしながら説明していきます。

以前タイピングソフトを作ったときのことです。タイピングソフトなので打ったキーが正しいか判定が必要です。そこで僕はmoji()という関数を作りました。まあ、命名からダメですが、とにかく最悪な関数です。

  • 関数がとにかくデカイ
  • 引数が多い
  • 同じ処理を繰り返している

順にポイントを見ていきましょう。

関数は小さく分割する

関数を小さくしましょう。

moji()では、

  1. 今打つべき文字が「ん」もしくは次の文字が小さい文字の時はgetRoman()を使い、ローマ字化する。
  2. その他の時はromanTable(ひらがな→ローマ字という変換テーブル)にある場合はそれを使う。
  3. キーが合っているのか判定する

という3つのことを行っていました。

まずそもそもローマ字への変換がgetRoman()に完全に任されていないですね。ローマ字への変換はgetRoman()へ完全に任せておけば再利用がしやすいですよね。テストもしやすいです。

そして、キーが合っているのかまで判定しています。2つ以上の処理を一つの関数に任せるのはテストするときに不便です。また、関数の命名もグダグダとなります。関数名だけで機能がわかるようにするためにも1つの処理に集中させて、関数を小さく分割しましょう。

引数が多い

引数が多いと、関数呼び出し時にいちいち確認しなければならずとてもストレスフルです。 多くても3つくらいにとどめると精神的に楽です。

実は、上で書いた「関数は小さく分割する」をすると、だいたい解消されます。引数が多くてなやんでる時はまず関数の処理を減らすようにしましょう。

DRY

DRYとはDon't Repeat Yourselfのことです。くり返しはなくしなさいということです。

moji()ではifで3パターンに分類してローマ字化をして、それぞれの中でキーが合っているのか判定していました。こうするよりも、ローマ字にしたあとでそのローマ字が合っているか判定するほうが良いです。キー判定部分は同じでしたので。

終わりに

こんな馬鹿なことしてないよ、と思った人も図星だった人も自分のコードを見直してみてください。きっと改善点が見つかると思います。