ソースコードの綺麗な書き方
今の職場のコードが結構アレ。
Rubyで、1メソッドで100行以上のコードが珍しくない状態。インデントも深い。
で、話をしていてだんだん原因が分かってきがする。
あまりにもイライラするので、ここに愚痴がてら書いておく。
こんなの基本なんだけどなぁ
原因
原因は、明らかに経験不足とコードを書くときにそこまで深く考えていない。
今回の場合RUbyを使用しているが、OOPとかそんなの関係ない。
クラス設計とかテストコード前提のリファクタリングとかそれ以前の問題で、Cの時と同じ考え方で十分綺麗になるはず。
Rubyとかの最近の言語だと基本綺麗に書けるので、こういう訓練とかってあまりやらないのかもしれない。そういうのが原因かも。
ということで、コードを綺麗にするいくつかパターンを書いてみよう(誰かの役に立つかもしれんし)。
テストコードも書かないし、エセ・リファクタリングとでも言っておこうか。
処理をこまめにメソッド(関数)に分ける
そもそも、人間ってそんなに記憶力が良くないし、長い文字列を覚えるのなんか1苦労だ。
世の中にはそういう人もいるんだろうが、少なくとも自分には出来ない。
古典的だがこの手法が使える、というかこういうのって基本だろうと思うんだが。
こんな風に、たいてい長いクラスでもいくつかの処理はわかれて書かれているはずだ。
def func #処理の説明 処理1 処理2 処理3 #処理の説明 処理4 処理5 #これ以降、長々と続く end
このような処理単位(処理1-3と処理4-5)で関数化して外だしするなり、Rubyならばメソッド内にメソッドが書けるので一旦処理を分割しておく。
1メソッドはどんなに長くても、特別な場合を除いて1画面内に収まるようにしておく。特別な場合というのはなにか長い初期データを作成する必要があるとかそういう場合だ。
def func0 処理1 処理2 処理3 end def func1 #処理の説明 処理4 処理5 end def func func1 func2 #これ以降、長々と続く end
これで、処理がかなり理解しやすくなったはずだ。
これだけで無駄な部分も見えてくるはずなので、ちょっとずつ整理していけば良い。
メソッド名は処理内容を的確に表すようにすること
Rubyは特にこの傾向が強い。便利なメソッドでもメソッド名がいまいちで取り込まれなかった場合も珍しくない。
名は体を表すというように本当に大事だ。
例えば"search_key_from_db"というメソッドがあったとしよう。dbから何かの"key"を検索するメソッドのようだ。
しかし実装が次のようになっていればどうだろう。keyがない場合には新しくkeyを作成している。おそらくDBに新しいデータを作成してkeyを取得しているように見える。
こんなのクソコードだ。searchならばDBを検索する処理だけあれば良い。searchなのにDBに書き込む処理があるなど言語道断だ。
def search_key_from_db #検索処理 key = search_key_func unless key key = make_key end return key end
条件検索分の中で処理をするな!
こういうパターンもよく見かける。
エラーチェックをしても問題がない場合のみ処理をしたいようだ。
unless error_check #なにか処理 end
基本的にメソッドは次のような流れになるのが一般的だ。
- 最初にエラーチェックをしてエラーならばreturn。
- 変数の初期化
- メイン処理
- 必要ならば、計算結果をreturn。
なので、つぎの様に処理をすればよい。
return if error_check #なにか処理
こういう問題のあるコードは処理のインデントが深くなるので、見づらくなる。
昔みたいに80文字までしか書けないという制約は現代には無いが、それでもインデントが深くなれば見辛い。
インデントはなるべく浅くするようにして、メソッドの処理数もできるだけ短く書くのが基本だ。
同じ処理は纏めろ!
えー、C、BASICとかの時代からの基本なんですがされていない場合が珍しくない
メソッドに纏めるのは基本だし、引数を変えてメソッドを複数連続で呼んでいるパターンも結構ある。
前者は無視するにしても、後者は時々見かける。
例えば、こんな感じだ
def func hoge(0) hoge(1) hoge(2) end
もう少し頭使おうよ。これで行けるでしょ。
def func [0,1,2].each{|x|hoge(x)} end
ちょっと頭使えば短く出来るコードをなぜか長々と書く場合が多い。
例外処理
Rubyの例外処理はこうだが、言語によっては構文が違うだけで基本は同じ構造のはず。
begin #なにか処理 rescue #エラー処理 end
例外処理での問題はいくつかパターンがある。
- 例外処理部分(beginの中)が異常に長い場合
- rescueで例外を指定しないパターン
- エラー処理でエラーを握りつぶして、何も対応しないパターン。つまり例外で処理を止めたくないだけで、問題が起きようと知ったことではないと考えている。
例外処理部分が異常に長い場合
これはどこの部分の処理で例外を止めたいのかがあやふやになるのでやめて欲しい。
ひどいとここがメソッド処理のほぼ全てになっている場合もある。
全体的にインデントが深くなるという問題もある。
rescueで例外を指定しないパターン
これは目的の例外以外もキャッチする場合があり、コードの問題発見が遅れる場合がある。最悪だ。
エラー処理でエラーを握りつぶして、何も対応しないパターン。
これも最悪だ。
そもそも、エラーはメソッドの戻り値で判断するパターンと、例外が発生するパターンがある。
戻り値で判断する場合は処理自体は出来たが、問題のあるパターンだ。
例外は、処理そのものが継続できない状態なので非常にまずいパターンです。
例外を無視すると、一見エラーが何も起こっていないように思えるが、実際には結構まずいエラーが起きている可能性がある。非常にまずいパターンだ。
処理が止まるとビジネス的に色々問題があるというのはよく分かる。だが、例外で処理が止まるほうがよっぽどマシ。
存在するメソッドを使わず、自作している
"同じ処理は纏めろ!"と通じる所もありますが、言語の構文を覚えれば結構楽に書ける場合が多いのを知らない人が多い。
例えばこんなのだ。
ret = [] array.each do |value| #何か処理 ret << 結果 end
こんなのmapにすれば、retが要らなくなりその分短くなる。map!にすればメモリ量も減るだろうし、Ruby本体で処理するので処理時間も早くなるはず。
array.map! do |value| #何か処理 end
Rubyにはこの辺り色々便利メソッドがあるので、ドキュメントを見ておくべきです。
"[].methods.sort"としてやればArrayのメソッド一覧が出てくるので、気になるメソッドは調べるとかすればよいでしょう。
Railsだと既存クラスを拡張しているので、さらに便利メソッドが増えていたりもします。
C時代でもなぜか、デフォルトで存在する関数を使わないとか結構見てきました。言語への理解が足りない事が原因だとは思うのですが。
まとめ
少し頭使えば、これくらいでもかなりソースは綺麗になりますがやっていない人が珍しくない。
仕事だとなぜかこういうコードが多く、理解できません。適当に書くのはプライベートでやって欲しいですね。
デザインパターンとか知らなくても、普段からこういう考え方をしていれば十分です。
ここまで出来れば、あとは色々開発手法を勉強したり、テストを書いたりすればよいでしょう。
Cプログラミング診断室の時代から本質は何も変わってない気がする。
いや、仕様書をあまり書かない分今の時代のほうがヒドいんじゃあないかな。