[Java]
スベテカンスウ
投稿者からのアピールポイント
何でもやってくれる関数って、便利ですよねぇ。もしバグってもメンテナンス能力を持った人が社内に必ず居るからね。
class Processor
{
public bool process_all(SomeHeavyParam param, SubParam subParam, boolean useProc23 = false)
{
proc1(param);
proc2(param, subParam);
if( useProc23 ) {
proc23(param); // 緊急対応
}
proc3(param);
proc4(param);
proc5(param);
proc6(param);
proc7(param);
proc8(param, subParam);
proc9(param);
proc10(param);
proc11(param);
proc12(param);
proc13(param, subParam);
proc14(param, subParam);
proc15(param, subParam);
proc16(param, subParam);
proc17(param, subParam);
proc18(param, subParam);
proc19(param, subParam);
proc20(param);
proc21(param);
proc22(param, subParam);
proc24(param);
proc25(param);
return true;
}
}
使い方ヒント: 「これは臭う」という行を見付けたら、各行の
をクリックしてマーキングしておきましょう(要Twitter OAuth認証)
コメント(14)
なんとなく怖さがわかってきた。 「proc1～2の動きが変だからproc23いれとけ。ついでに引数も追加」ってことで、バグ修正のたびに引数と関数が増えてく？ 結果proc25まで増えてしまったとか・・・
やさしさというより、proc23を入れてもまだおかしいから、後でproc24,25を追加しただけでは？空行は時間経過を物語っているのかと。
SomeHeavyParam の中にはprocNoみたいなパラメータがあって各関数の頭には if(param.procNo != 4) return; みたいな文が入っているに違いない…
おゎ、知らぬ間にコメントがたくさん...。おおむね @hardtimes777 さんのおっしゃる通りです。
proc1 〜 proc25 は「その中でやっていること」をさしおいて「process_all の読みやすさ」だけを考慮して設計しているという点がウンコです。そのせいで、後から proc2 と proc3 の間に修正を入れたくなっただけで「process_all の読みやすさ」さえもが破綻してしまっています。
また「process_all の読みやすさ」だけを強調したいがために各 proc の引数にバカでかいパラメータを受け取るようインターフェイスを共通化しており、proc1 〜 proc25 を process_all 以外の場所で再利用することすら難しくしてしまっています。さらに、後になって SomeHeavyParam だけでは足りなくなってきたため SubParam を追加定義しており、意味の無い型名にせざるを得ない苦しみがあります。
「何でもやってくれる関数って、便利ですよねぇ。もしバグってもメンテナンス能力を持った人が社内に必ず居るからね。」というのはつまり、この process_all 関数がアプリケーションの色んなところから呼ばれているためリファクタリングが難しく、この関数はやむを得ず肥大化してきた経緯がある、ということを予想してもらうための添え書きです。それを示唆するのがデフォルト引数 useProc23 です。useProc23 を追加したプログラマは、ここをデフォルト引数にすることで過去のソースについて何ら心配しなくてよかったわけです。明らかに付け足しであるかのような引数にデフォルト値を指定するのは汚いソースを醸成するための基本テクです。
そして何より、@k5n6 さんがおっしゃるようにオブジェクト指向っぽさがカケラも感じられないコトが、もはや前提レヴェルになってしまっています。最後に、この関数が絶対 true を返すことにも注目です。意味の無い戻り値をとりあえず設定するのもウンコードっぽいですよね。
もうちっと、冒頭で↑↑のエッセンスを語ってほしかったなぁ。
しかしhardtimes777さん、エスパー能力高すぎ。すごい。
てか、これ良く見るとJavaのコードじゃないね。
コメント投稿には、twitter認証が必要です。
Twitter認証
このサンプルだけ見て積極的に批判する理由は見つからないなぁ。要は関数の内容次第じゃないの。
1メソッドの中に何千行も延々と処理を書かれるくらいなら、この方が何倍もマシ。
あと、
> もしバグってもメンテナンス能力を持った人が社内に必ず居るからね。
もよく分からなかった。なにか僕らの知らない前提を置いた上で投稿してるのだろうか。