まえがき
今回書く内容は、ある程度経験あるエンジニアでも、陥りがちなものに絞って書いてみたつもりですので、[重複コードは書かない]などの超あたりまえの事は書いていません。
2017/03/16
最近よく見られてそうなので１つ追記[そもそも継承するな!!!]
そもそも継承するな!!!
継承するのは、どうしようもない場合のみにしてください。
その前に、strategyパターンや、compositeパターンなどの他のやり方を考慮してもなお、継承するのが妥当である場合のみにしてください。
基本的に継承しないほうが、スケーラブルだし、テストコードも容易にかけます。
継承はis-a関係
「あー、継承ね。はいはい」で飛ばしてんじゃねーよ。
いやマジで!!!
ほぼ全てのエンジニアは[is-a]が何か知っています。
というのも全てのオブジェクト思考の書籍には出てくる概念だからです。
しかし、私の経験上この概念を疎かにしている人が多過ぎる。
[現場でのある一コマ]
Aさん：「このクラスとこのクラスのメソッド全く同じじゃん」
Bさん：「あー、じゃあ親クラス作ってまとめたら重複コード減らせるね」
私：「ふざけんな！！！」
となるわけですね。
いや、まあとっかかりとしては良いと思います。
しかし継承で一番重要な概念である[is-a]が抜けてたら絶対ダメです。
[is-a]は飾りでも、初心者用の言葉でもありません、実装で使われるべき概念です。
これを疎かにするからわけのわからんクラスができる。
でもはっきり言って[is-a]って抽象的過ぎて結構実装レベルで落とし込むのって難しいというか、個人差が出るというか・・・
なので１つ具体的な判断の仕方をご紹介するので、参考にいただければと思います。
「親クラスで使っているプロパティ、メソッドを全て使える状態であるか」
勘違いしないでほしいのは、使っているのではなく、使える状態であるかということです。
もちろん使わないこともあると思うので、使おうと思えば使える状態にあるかで判断してください。
これができていれば[is-a]関係と言っていいと思います。
なのでまず、子クラスを作る場合はこの状態であるかを見てください。
メソッドのインプットとアウトプットを明確に
とにかくこれは守れ！
守れないならテストを書け！(守らざるをえないから)
マジin,outが曖昧なメソッド多過ぎる。
しかもこれを改修するのつらすぎる、マジ守って欲しい。
テストも書けない。マジ辛い
あー。
特に他に書くことないですが、「このメソッドのインプットとアウトプット何？」と聞かれた時に答えれるメソッドにしてください。
もちょっと具体的に、以下の処理を一つのメソッドでやるなということです。
[何かの計算をした→計算結果を保存]
計算結果を返して、呼び出し元で保存しろということです。
スタートがあればストップもある
これも結構見かけます。
なんでスタートがあんのにストップねーのよ。
スタートとストップだけじゃないのですが、対になるべきメソッドってありますよね。
スタートはインスタンスのコンストラクタでやっていて、ストップは外からさせるとかいうクラスがあったりしませんか？
とにかくコードには一貫性を持たせましょう。
狭く強く優しく
公開範囲は狭く、制限は強く、優しくありたい
兎に角メソッドは全部privateにする。そのあと必要なものだけ公開するということにしてください。
クラス内でしか使ってないものを公開しているとホント見にくくてしゃーない。
プロパティもまずはreadOnlyでそれでもダメなら書き込みも。
変数も全てconstつける、メソッドの引数もconstをつける。
制限できるところは全て制限する。
コールバックは、ロジックと絡めない。
何かロジックが書いてあるメソッドの中でコールバックを呼ばないでください。
これは、呼び出し元に自分がコールバックを返す場合です。
子から親に返す部分をロジックと絡めないということです。
(つぶやき)
ロジックが書いてない、値の受け渡しだけのコードの名前って何でしたっけ？何か名前があったのですが忘れたので、分かる方コメントに書いてください。
2017/03/16
はてブに、「わかんない」的なこと書いてもらったので、見直したら、「そうだね」と思ったので以下追記。
当時怒りのあまり、雑に書いていましたwww
例えば、
計算1,計算2,計算3があったとします。
コールバックで「計算1 + 計算2 + 計算3」を返したいとする。
それを一つのメソッドで
//コールバックなので非同期で返してます。
全計算結果{
return [計算1 + 計算2 + 計算3]
}
ではなく
計算1結果{
return 計算1
}
計算２結果{
return 計算２
}
計算３結果{
return 計算３
}
//コールバックなので非同期で返してます。
全計算結果{
return [計算1結果 + 計算２結果 + 計算3結果]
}
と書きなさいということです。
なぜかって？
どっちがテストコード書きやすいかってことに尽きますかねーー。
コールバックは非同期なので、そこにロジックを絡めてるとテストが辛いですよね？
相互参照は絶対ダメ
もう力尽きました、書いてある通りです。
私が見たコードでは、[has-a]関係での親に当たるインスタンスがシングルトンになっていて、子供のインスタンスがシングルトンで取得した親インスタンスのプロパティに自分自身を突っ込んでいました。
親を直接参照するプロパティを持ってなければいいてもんじゃねーよ。
これは相互参照じゃーー
コメントは綺麗に書こうとするな。思いを込めろ！！！
先日私が見たコメントでこんなものがありました。
//メインスレッドで実行
これを見て私は「なぜ？」と呟きました。
そう一言で言うならば
「なぜ？」と疑問を持たれるコメントを残すな!!
ということです。
今回のコメントの良い例としてはこんな感じです。
//呼び出し先で、描画処理があるためメインスレッドで呼び出し。
次は思いを込めたコメント例
//よくわかんないけど、メインスレッドで呼び出したら、クラッシュしなくなった。
このコメントを見ると、エンジニアのレベル感も伝わるし、修正しても良いのだなとわかるので、原因がわかる人は修正することができます。
もう一度以下のコメントを見てもらってどうでしょうか？
//メインスレッドで実行
なぜこうしたか分からない以上は手を加えることもできません。
こういったらコメントのせいで、よくわからないコードが増えていくわけですね。
追記 2016年08月16日
この投稿を見た、前の現場の方から私の書いていたコメントが届いたので、ここで紹介しますw
# koitaro 2015/09/14
# Thread.current[:request]にurlを突っ込んで、それにstepが含まれているかで
# 判定を分けているが、これはいわゆるグルーバル変数なので、このような書き方は良くない
# そのため、どうにかリファクタリングをしようと試みたが、深くネストされて使われている
# ため安易に改善できず断念した。
# 勇者が現れる日を待つ