jfluteの日記 このページをアンテナに追加

2016-02-03

既存コードの甘い匂い (悪意なきチグハグコードの誕生)

|

前提として、
しっかりとコーディング規約やコーディング手順などが
整備されている現場ではあまり関係ないかもしれません。

そういうのを整備して実践していことが難しい現場、
スタートアップからインクリメンタル開発を経て、
成長していくサービスを長期間作り上げていく
事業会社が主なターゲットの話かもしれません。

既存コードに if 文がありました

さて、あなたが開発現場に途中から参画しました。
すでに A と B という別の人が作った画面があります。
あなたは C を作ります。
C は A と B と似ています。

「さあ、作ってください」

と言われました。どうしますか?
...

まあ、普通に考えたら、A と B を参考に作りますよね。
ここに甘い匂いがします。

A と B には、とある定型の if 文による制御がありました。
C では一見、要らなそうに見えますが複雑でよくわからない。

C でも if 文を入れますか?
(念のため、入れておきますか?)

...

「Yes」の痕跡をよく見かけます。

でも、実はその if は、

A と B だからこそ必要で、
C では必要ではなかった

そんな結末よくあります。

たかだか if 文と思われるかもしれませんが、
後で見た人がコードを解析するときに、
その業務での本質的なロジックを見極めるのに
ボディーブローのように効くノイズになります。

似たようなものが何個も何個もあるとしたら?
チリも積もって、分析コストはどんどん高くなります。

...

さて、D を作る新しい人が入ってきました。
A と B と C を参考に D を作り始めます。

そう、

めでたく C は既存コードになりました

その if 文、確実に D では不要なのですが...
果たして、D でその if 文は入ってしまうでしょうか? 

...
...
...

処理を追加するとかしないとかの話だけではありません。

A と B で、とある特徴的な書き方のコードがありました。
それはあまり「良いとは思えないコード」になっています。
もっと良い書き方があるはずなのですが、
C では、A と B のやり方を真似ますか?

...

とまあ、似たようなことはたくさん。

...そう
とにかく、

既存コード

昨今のレビューの定型パターン

これと似たような状況を、細かく細かくたくさん見かけます。
その if 文を入れた人のレビューをします。
jflute「その if 文、必要?というか、
C だったら絶対に true にならないけど、
万が一 true になって動いちゃってもつらくない?」

作った人「ああ、でも既存に合わせて作ったので...」

jflute「おおぅ、ちょっとその実装、さすがにわかりづらいね、
もっと、こういう風に書けばスッキリできるはずだけど」

作った人「ああ、でも既存に合わせて作ったので...」
昨今、このレビューやり取りを繰り返すことが頻繁にあります。
さすがにこのレビュー、最強につまらないです(><

だんだん、ぼくもわかってきて、

「恐らく、これは既存に合わせたんですね、
うーむー、そうですねぇ...」

と、先に言って悩んじゃうみたいな。
どう指摘すればいいんだか?って。

恐らく、その既存コードを書いた人も、
「既存がそうなっていたから」
と言っていたに違いない。

しかも、プログラム経験が浅い人だけというわけではなく、
プログラムはわりと書く実力のある人でも発生します。
そこがちょっと不思議に思うところ。

既存コードが良いコードであれば発生しないかというと、
必ずしもそういうわけでもなかったりも。
既存コードは既存コードの問題領域で最高の解決をしていても、
違う問題領域で同じ解決してもフィットしなければチグハグなだけ。

よく自分は「ソース読め読め」言う立場ではありますが、
なんか、
「だったらまだ既存コード読まない方がいいんじゃね?」
って思ったりすることも。。。

どういうメカニズムか?

ダメなコードによる「むっきゃー」という、
ブログやつぶやきは、ずぅぅっぅっと昔から収まりません。

単純に「頑張ったけど良いコードが書けなかった」
まあ、それはそれで Problem ではありますが、
わかりやすい問題ではあります。
本人も頑張ってるなら徐々に良いコード
書けるようになるだろうし、
そのために応援できることはいくらでもある。

でも、そうではない。

誰も悪意のある人がいないのに

チグハグなコードがどんどん生まれていく。
でも、結果として良いコードには仕上がらず、
あとでそのコードに関わる人がストレスを感じるという、
ちょっと悲しい出来事。

...

ここで悪意のがある人がいないという言葉に
ひっかかる人もいるでしょう。
悪意あるじゃん!そいつ全然だめじゃん!
って。

まあもちろん、C と D を作った人が、
仕事人として足りた行動をしているかは疑問です。
これは後で述べます。
が、ここで言いたいのは、
「うへへ、悪いコードを書いて迷惑かけてやるぜぇ」
って気持ちで書いているわけではないということ。
(まあ、さすがにそんな人いないけど...)

じゃあ、どういう気持ちでそうやってしまったんでしょう?
本来良いコードを書く実力がある人だったりもするのに。
また、書いた本人だけの問題なのでしょうか?
そこを紐解いていかないと問題は解決しないと考えました。

その人にとってはその人の論理で
良かれと思ってやっている。
ただ、それが合っているかどうか?

がポイント。

月並みメカニズム

{A}
既存コードに合わせることで、
コードの統一性に貢献をする。

確かに、良心から来るものではあります。
そういう貢献の精神は立派です。
これ自体は間違った考え方ではないです。

ただ、それが結果としてGoodになるには、
既存コードが統一の規範に則っているケースに限ります。
そうでなければ「ダメなコードで統一を図る」のに
貢献しているだけになってしまいます。

また、既存コードではフィットするコードも、
新しい画面でフィットするとは限りません。
良い既存コードからチグハグなコードも生まれます。

プログラマーがそれを確認する行動が足りてないのが一つ。
その辺のニュアンスを新しい人に伝えていない周りの人の
配慮が足りてないのが一つ。
そういうポリシーが固まっていない現場全体の問題が一つ。
既存コードを書いた人の実力不足が一つ。
既存コードを書いた人が仕方なしにサバイバルコードを
書いてしまったのであれば、それをコメントにして、
後でコードを読む人のために行動をしていないのが一つ。

...
...

{B}
時間がないので、
既存コードを参考にして早く作り上げる。

これも確かに、良心から来るものではあります。
これ自体は間違った考え方ではないです。

ただ、もの作りはそんな単純ではなく、
特にサービス系では、
一度作ったものの上に新しくコードを乗せていきます。
その土台がグラグラしていると、
乗せていくときのパフォーマンスが悪くなります。

大抵、新しい現場に入ったら、
特にパートナーさんとかだと、
最初はその実力を疑われてますから、
(本人は疑われてると思ってしまいますから)
「早く成果を上げないといけない」
という精神状態にあります。

「作って」と言われたら、
とにかく早く動くもの作って納品する、
というのがデフォルト行動になると考えます。
また、与えられた仕事の業務的な優先度がわかりません。
なので、早く作ること以外に貢献が思い付きません。

でも大抵は、新しく来たばっかりの人にいきなり
業務の最優先課題を与えることは、
そんなにはないかと思います(時々あるけど...)。
まずは、
「この現場のやり方を学んでもらう」
「よそでの経験を活かして良いフィードバックをもらう」
そういうことが暗黙に求められています。
でも、あまりそういうことを伝える人はいないので、
本人の精神状態とはだいぶ乖離が発生します。

それにより、とにかく、

既存コードを真似まくって

超特急で作り上げます。一瞬、速くてすごい!
ってことになるのですが、
A, B から C が作られ、C から D が作られ...
の路線にそのまま乗っかってしまっただけだった、
ということが往々にしてあります。

...
...

{C}
フレームワークのコード読まない。

まあ、月並みなので、多くは語りませんが、
ちょっと、ctrl+click して読めばそれが要らない、
もしくは、使い方が間違っていることはわかるのに、
読まないのでよくわからず真似ておしまい。

既存コードは読むけど、
commonのコードは全く読まない

単純な話なのに不思議な話ですね。

...
...

{D}
経験不足で判断基準が既存コードしかない

これは、若い人に多いかもしれません。
そもそもどう書けばいいのかわからないから、
既存コードしか頼るものがない(ように思ってしまう)。

また、仕事以外でコードを書くことがない人だと、
陥りやすいかもしれません。
短いプログラマー人生の中で与えられる仕事の
多様性というのは、非常に限りがあります。
どういうコードが良いのか?これをどう書くか?
という判断基準が養われることなく
育ってしまうこともあるでしょう。
その状態で新しいプロジェクトに入ったら
確かに判断基準が既存コードしかないかもしれません。

単純な話なのに難しい話ですね。

でも、一番のメカニズムはこれ

{S}
既存コードに合わせていれば、
変なコードでも自分に責任はない。

逆に言うと、

既存コードと違うことをやって、
バグを出したり開発が遅れたりすると、
大きな自分の責任になる

という心理。

わりと本能的なものでもあり、
本人が意識的にそう思ってなくても、
自然とこの論理で行動してしまいがちです。
これは、相当にデカいと思っています。

jfluteもその気持ちわかります。
わかるのに、まあ性格が KY だから、
「えーい俺こう書いちゃお」ってなっちゃうんですが、
まあ、正常な人だったら、
たぶんそんなリスクを背負わないです。

少なくとも、デフォルトの精神状態はそうだと思います。
フリーランスやパートナーであればさらになおさら。

インクリメンタル開発だとほぼ

たぶん「かっちりSI」だと、
あまりそういうことが問題になることは少ないのかもしれません。
もう書き方から何から何まで細かく決められていれば、
新しく入ってきた人も迷わない。
逆に、既存コードを読みさえしないのかもしれません。
仮にチグハグになったとしても、
それを修正する人がいなければ問題は顕在化しないし、
ドキュメントも豊富にあります。

...

スタートアップ後のインクリメンタル開発だと、
そもそもルールもなく、スピード重視で、
コードを積み上げていくことが前提です。
そして、人も後からどんどん五月雨で入り、
入れ替わってもいきます。
成長期の事業会社だと、プロパーエンジニアを
集めるのがなかなか困難でもあったりするので、
すると、フリーランスなどのパートナーさんに
お願いすることが多くなり、
人の入れ替えはさらに激しくなっていきます。

...

この辺は、以前のブログ記事が詳しいです。
 => SIとスタートアップの違いを知ろう

多くの事業会社において、
「既存コードの連鎖」
が発生しているのではないでしょうか?

サービス開発のコードのカオス化、レガシー化、
その原因の大きな割合を占める話であると思っています。
極端に言うと、言語のバージョンやフレームワークが古いとか、
この話に比べれば全然小さなものと言えるでしょう。
(もちろん新しくしたいけどね…
ただ、問題の重要度という意味で比べれば)

この話を解決する道がなければ、
いくら枠組み (ガラ) が新しくなっても、
またまた、
「誰もがさわりたがらないコードの塊」
になることには変わりはないのです。

そして、優秀な人から、その現場を去っていく...

どうすればいいの?

って、ここまで書いておいて、
その答えは見つかっていません。
でも、そのメカニズムを知ることが大事だと考えました。

(その人も行動が足りてはいないけど...)
チグハグコードを書いた人が悪い、
だけでは解決しない問題

ということがわかるからです。
チグハグコードを愚痴っても、
チグハグコードを書いた人を吊るし上げても、
たぶん、この既存コードの連鎖がある限り、なくならない。

というか、吊るし上げようにも誰が悪いのが
いまいちよくわからないってところ。

かっちりSIのようにルールを作ればOK?
いや、なかなか成長期の事業会社だと難しい。
もちろん、努力はしますが、
細かいところまでかっちりとまではいけないでしょう。
(そもそも、そういうルールはあまり作りたくないし)

少なくとも、
一つのソリューションで解決できるとは思えないのです。

o 単純に既存コードを書いた人のスキル
o 仕方ない既存コードを書いた人の気づかい
o 既存コードを読む人の行動
o 周りの人のフォローイング
o 新しく入ってきた人のオリエンテーション
o そもそもの現場のポリシーの明文化
o いろいろ

【追記】
Twitterで「ボーイスカウト・ルール」という
コメントを頂きました。ありがとうございます。
「来た時よりも美しく」
そういった文化を浸透させていくのもアプローチの一つですね。
そして、新しい人にそのルールが推奨されてることを伝えて、
改善に遠慮することがないように配慮してあげたいですね。

プロフェッショナルとして

まあでも、とりあえずぼくが、
既存コードの連鎖を目撃してよく言っているのは...

既存コードに合わせる姿勢自体は良いとして、
「既存コードに合わせて終了」の姿勢は、

プロフェッショナルとして、どうでしょう?
お金もらって仕事をしていますから。

よくわからない if 文があるなら...

これを真似ていいのかどうか?
 => さらに奥のソースを読んで判断すればいい
 => わからなければ周りに聞けばいい
 => ポリシーがわからなければ周りに聞けばいい
 => どうしても真似るんであれば真似たことを明示すればいい

ぐちゃぐちゃでも動けばOKの急ぎ仕事なのかどうか?
 => 聞けばいい

しっかりと、この行動をして、
ある程度既存コードに習うところはあるにしても、
いま作っている画面における問題領域をしっかり把握し、

考えて

その画面に適した実装の成果をあげてくれる人も
実際にいます。プロですね。

それをやらず「自分でも説明のできないコードを提出する」
というのは価値の高い仕事でしょうか?

まあ、程度の問題ではあります。
時間の限度もあるし、それ以外の問題もあったりするから。
「既存コードに合わせる」は別に禁句なわけじゃないから。
だから、程度の問題。境界線はない、だから難しい。
(聞いたら聞いたでたらい回しされて、
時間めっちゃ食ってとか確かにあったりもするし、
聞いたところで答えが出てこなかったりもするから)

でも、ちょっと姿勢を変えるだけで、
その成果はだいぶ変わるんじゃないかなって。

プログラマーとしてのプロ意識

解決策になってないですが、
大切にしたいところです。

f:id:jflute:20160107153451j:image
トラックバック - http://d.hatena.ne.jp/jflute/20160203/existingcode