ふつーのプログラマです。主に企業内Webシステムの要件定義から保守まで何でもやってる、ふつーのプログラマです。

高慢と偏見(3)コードレビューは踊る

»

 次の日から、三浦マネージャによる新体制がスタートした。

 もちろん、急に何かが変わったわけではない。これが開発初期だったら、それまでの作業をなかったことにしてゼロからスタートという事態もありえただろうけど、機能数でいえば3割ほど実装済みのものを捨てることは、さすがの三浦マネージャもできなかったようだ。

 それでも三浦マネージャの「方針」は早速実施され、私たちの開発に負担を強いることになった。

 まず、コードレビュー。

 最初、三浦マネージャは週2回、各3時間で実行するつもりだったらしい。しかも全員参加で。

 これには平良さんが難色を示し、かなり強く異議を唱えた。開発初期ならともかく、ここまで開発が進んだ状態で、しかもスケジュールが押している時に、週6時間も実装とは別に時間を取られるのはバカげていると、私でも分かる。

 結局、三浦マネージャは渋々ながら週1回、2時間で妥協、ただし全員参加だけは頑として譲らなかった、とはアツコさん情報である。

 金曜日の13時ジャストに、第1回のコードレビューが実施された。よりによってこんな眠たい時間にやらなくても、とみんなブツブツ言っていた。もちろん三浦マネージャに聞こえない程度の声で。

 全員にコードのプリントアウトが配られた。マスタメンテナンス画面が1つ、夜間バッチ処理が1つ。HTMLソース、バッチを起動するシェルスクリプトなどは省略されている。

 「これ、どういう基準で選んだんでしょうね」

 「さあね。どうせアミダか何かでしょ」

 私のささやきに、アツコさんは不機嫌そうに答えた。自分の時間をムダにされることを何よりも嫌うアツコさんだった。

 三浦マネージャは、全員に向き合うデスクに座った。なぜか薄笑いを浮かべ、メガネ越しにプリントアウトを凝視しながら、早くも赤ペンで何やら書き込みをしている。

 メンテナンス画面からレビューが始まった。

 コーディングした富永さんというエンジニアが全員に、というより三浦マネージャのために機能概要を説明しているのを聞きながら、私はコードをざっと眺めた。

 新部品調達システムは、WebフレームワークにSeasarプロジェクトのTeeda(ティーダ)を採用している。当初は、Struts技術者の方が人を集めやすいため、同じSeasarプロジェクトのSAStrutsが有力だったのだが、設計段階に参加していたシステム部の社員が「Struts はもう古いし、JSPを使うとスクリプトレットが増えて、ロジックが分散してMVCの概念が崩れる」とか何とか主張した、とのことだった。Teedaの HTMLテンプレートが画面UI担当のデザイナに好まれたこともあって、最終的にはTeedaに決まった。ちなみにその社員さんはとっくの昔に、このプロジェクトからいなくなっていた。理由は誰も知らない。

 平良さんもパフォーマンスなどの点からSAStrutsを支持した1人だったが、Teedaの経験もあったため、基本となるコーディングのパターンは彼が決めた。

 基本的に登録/更新系の画面ほとんどは、メイン画面、確認画面、完了画面の3つを持つ。このうち、完了画面はシステムで共通となっていて完了メッセージをセットして遷移するだけだから、特殊なケースを除いては個別に作成しない。

 例えば、xxxxxx という画面を構成するのは、

  1. xxxxxx.html……メイン画面のHTML
  2. confirmXxxxxx.html……確認画面のHTML
  3. XxxxxxAbstractPage.class……メイン画面、確認画面すべての項目に対応するプロパティを持つページクラス
  4. XxxxxxPage.class……メイン画面HTMLに対応するページクラス
  5. XxxxxxAction.class……メイン画面のアクションクラス
  6. ConfirmXxxxxxPage.class……確認画面HTMLに対応するページクラス
  7. ConfirmXxxxxxAction.class……確認画面のアクションクラス
  8. XxxxxxLogic.clsss……ロジッククラス

の8個となる。

 多いように見えるが、確認画面(confirmXxxxxx.html)は、メイン画面とレイアウトがほぼ同じで、すべての項目が readonlyになっているだけ。XxxxxxPage.classと、ConfirmXxxxxxPage.classは、 XxxxxxAbstractPage.classのサブクラスで、項目そのものはXxxxxxAbstractPage.classに記述するから中身はない。Actionクラスは、基本的にロジックを呼ぶだけなので、最初に記述してしまえばほとんど触ることはない。

 つまり、実質的にコーディングするのは、xxxxxx.html、XxxxxxAbstractPage.class、XxxxxxLogic.clsssの3つでいいことになる。

 「……機能の説明は以上です」富永さんが口早に説明を終えた。明らかに時間の無駄だと思っている様子がありありだ。

 「はい、ありがとう」三浦マネージャが薄笑いを浮かべたまま言った。「じゃ、誰か質問はあるかな?」

 誰も何も言わない。言うべきことが見つけられないというべきか。

 そもそも、このプロジェクトのように、似たような画面をたくさん作るシステムの場合、基本的なパターンさえ固まっていれば、それなりのスキルがあるエンジニアなら誰がコーディングしても似たようなところに落ち着くものだ。今までコードレビューの必要がほとんどなく、あったとしても新しくプロジェクトに参加したメンバーに経験者が教えれば済んでいた、という事実がそれを裏付けしている。

 私も富永さんの説明に従ってロジッククラスのソースを追っていたが、特に意見すべき点を発見できなかった。必要十分にしてムダがない。PropertyUtilsなどを適切に使ってコーディング量を減らしているし、例外処理も抜けていない。

 強いて言えば、不要なコメントが目立つと指摘できないこともないが、これが富永さんのスタイルなのだろうから、特に問題とは思わない。たぶん、この部屋にいるメンバーなら誰でも同じ意見であるに違いない。

 ところが、私の考えは相当甘かったようだった。というか、三浦さんの注意はもっと別の場所に引きつけられていたのだった。

 「誰もいないか? おかしいな。じゃ、私から聞こうか」三浦マネージャは、メガネを持ち上げて手元のプリントアウトを注視した。「ええと16行目の、

List selectedPartsList = new ArrayList ();

だけどね、なんでList で宣言してるの? ArrayListなんでしょ?」

 「は?」

 ――え、そこ?

 私は意味が分からず、16行目を見直した。別におかしな書き方ではない。

 「は? じゃないよ。なんで?」

 富永さんはきょとんとしていたが、おそるおそる聞いた。

 「あの、どこがおかしいんでしょうか?」

 「どこがって、そんなことも分からないのかなあ? なんでListで宣言してるの?」

 「いや、それが普通だと思うのですが……」

 普通というのは言い過ぎかもしれない。この場合、ArrayListで宣言する人は確かにいるし、それが間違いとまでは言い切れないから。

 ただ、私はこういう場合、Listで宣言するように教わったし、それを疑問に感じたことはない。インターフェイスや抽象クラスを使うというのは定石だから、「なぜListで宣言しない?」と怒る人がいることは知っている。

 だが、まさか「なぜListで宣言しているのか?」と非難する人がいるとは想像しなかった。

 「だからね」三浦マネージャは子供に言い聞かせるようにゆっくり言った。「どうしてわざわざ違う型で宣言しなくちゃいけないの? おかしいでしょ、普通? 整数使うのにfloatとか使う? 使わないでしょ?」

 ――いやいやいや、それは違うでしょう。型の違いと、インターフェイスと具象クラスの違いをごっちゃにしてます。

 「ArrayListを使うんなら、素直にArrayListで宣言すればいいじゃない。どうしてわざわざこういう分かりにくいことをするかなあ。カッコつけか?」

 ――インターフェイス使うとカッコいいんですか?

 私の脳裏に、先月、隣の部屋から聞こえてきた言葉がよみがえった。

 「オブジェクト指向など実業務では使いものにならない!」

 三浦マネージャが無能なはずがない。無能な人がK自動車で管理職になれるはずがないのだ。この会社はかなり厳しい成果主義で昇進・昇給が決まるので有名だ。TOEICで700点以上取らないと絶対に管理職にはなれないし、年に1回は職務内容に応じた技能維持研修が義務付けられている。

 だから、最初にその言葉を聞いた時、実は何か根拠があるのかもしれない、と考え直したことがある。隣の講師の人は、オブジェクト指向を徹底的に使い倒し、デザインパターンもリファクタリングも完全にマスターし、そのうえであえて「使いものにならない!」とアンチテーゼを投げかけることによって、ゆとり世代の新人たちを刺激しようとしたのかもしれないと。アツコさんは、それを聞いて大笑いしていたが。

 平良さんが手を挙げた。

 「お言葉ですが、この記述はごく一般的なものだと思います」

 「一般的なんて誰が決めたのかね? 私はArrayList で宣言する方が一般的だと思うよ」

 「いえ、しかし、仕様を実装から分離するのはオブジェクト指向の基礎ではないでしょうか」

 三浦マネージャは得たりとばかりに笑った。

 「ほらね、そういう意味のないことばかりだから、オブジェクト指向は現場で役に立たないなんて言われるんだよ」

 確信犯だ、と私は思った。

 この人はまじめに言ってるとしか思えない。アンチテーゼとかそんなものではなく、まじめにオブジェクト指向が実務では使えないと信じ込んでいるのだ。

 「意味がないとはどういう意味でしょうか?」

 冷静沈着な平良さんも、さすがにイライラしているのが分かる。

 「私の長い経験からするとね」三浦マネージャは「長い」を強調した。「ポリモリズムでインスタンス生成を行っている例なんか、見たことがないよ。Webで検索してみても少ないしね。みんなそんなもの使ってないんだよ」

 ――あんたの経験って何? 適当にググってみるのが経験? しかも、今度はポリモリズムって。

 「このプロジェクトでは、全メンバーが使っていますが」

 「ああ、そのようだね、このコードを見れば分かるよ。そろそろ正しいやり方に修正しようじゃないかね」

 「正しいやり方?」

 「インターフェイスとかポリフォリズムとか、そういうのをカッコつけて使わないってことだよ」

 三浦マネージャは明らかに上から目線で言い放った。しかも、また単語を言い間違えている。わざとか?

 「インターフェイスを使うのは別にカッコつけているからではありません。変更に強いプログラムにするためです。オブジェクトの振る舞いを実装から分離することで……」

 「ああ、そういう細かい理屈はいいからいいから。とにかくインターフェイスなんか使う必要ないの」

 平良さんはムッとしたように口を閉じたが、すぐにまた手を挙げた。

 「1つだけよろしいでしょうか」

 「なんだね」

 「ポリモーフィズムです。ポリモリズムでもポリフォリズムでもありません」

 三浦マネージャの顔色が一瞬変わったが、すぐに薄ら笑いを浮かべた。

 「ああ、そう。どうでもいい些末(さまつ)な事柄は記憶しないことにしてるんだよ。じゃあ、次に行こうか」

 (続く)

 この物語はフィクションです。実在する団体名、個人とは一切関係ありません。また、特定の技術・製品の優位性などを主張するものではありません。

Comment(32)

コメント

anonymous

第3回キタコレ

サイドストーリーを知ってると楽しめますなあ。

rinnna

とても身につまされる話で、マジマジとみてしまいました。
制御系なので若干違いますが、まるで、うちの会社を見ているようで…

レモンT

「私たちは今夢を見ているのよ!このプロジェクト自体が夢の中の出来事なのよ!!」という訳には行きませんよね、やっぱり(てかそれだと最後プロジェクト崩壊だよ)。
 ともあれ、同志狂茶党員の更なるご活躍を(青いシャンパンでもいただきながら)今後とも楽しみにしています。
 ……この業界の不条理さのほうがよほどMad Tea Partyがごとしだという気もしますけど。
 それでは。

リーベルG

トロツキーさん、どうも。
「僕はついに成功しないと判っていても、なおプロジェクトの夢を”オブジェクト指向”に求めたいのだ」
ってとこですかね。

次回をお楽しみに。

九古

List型変数でArrayListのインスタンスを格納するのって、
普通にやってますけど、改めて理由を訊かれると即答できませんねえ。
ArrayList型変数に格納したら不都合が生じるパターンというのもちょっと思いつきませんし。
敢えて言うなら、
使いたいのはあくまでもList型であって、
ArrayListはその実装に過ぎないから、というのが理由なんですかね。

Fufuhu

”TOEICで700点は優秀”は冗談かもしれませんが、
700点じゃ実務じゃあんまり役に立ちません。
700点くらいなら学生でもちょっとましなレベルなら普通にとれますよ。
TOEICはライティングとスピーキングが無いので英語に対して受身の能力
(リスニングとリーディング)しか身につかないのでコミュニケーション能力は
測れないんですよ。
(最近はスピーキングとライティングも測ってくれるテストが別枠でできたようですが)

ListでArrayListを受けるのは
Listだと実際のインスタンスがLinkedListでもArrayListでも
何でもどんと来いってできるからでは?

List list = new ArrayList();
ってしてたのを
List list = new LinkedList();
って書き換えても”他の部分に影響が出にくい”ってことでは?
まあ、来年からそういった業界で働く予定の学生の戯れ言ですから聞き流して下さい。

おそらく三浦さんって方はあんまり大人数でやるような大きなプロジェクトを
任されたことがないんじゃないかなぁ…
一人でやっててもある程度プログラムが大きくなってくるとポリモーフィズムとか
インタフェースとかの考え方は後々の修正やメンテナンスを考慮すると非常に有用ってのは
すぐに理解できそうなんですがねぇ…

rinnna

Fufuhu さん

>List list = new ArrayList();
>ってしてたのを
>List list = new LinkedList();
>って書き換えても”他の部分に影響が出にくい”ってことでは?
>まあ、来年からそういった業界で働く予定の学生の戯れ言ですから聞き流して下さい。

自分の場合、Listに代入すると、使用するときに、Listとしてふるまう
機能以外の箇所を見なくて済むから、範囲を狭められて、便利ということに
思い至って、なるべく、直接ArrayListとかに代入しなくなりました。

オブジェクト指向言語はあまり得意ではないのですが、やってくと
普通の手続型言語(C言語)使う時にいい影響があるので、勉強になります。

flatline

>まあ、来年からそういった業界で働く予定の学生の戯れ言ですから聞き流して下さい。

地獄へようこそ。
ってのは、半分ぐらい冗談ですが。

インターフェースで宣言するのは、実装の切り替えが簡単にできるという利点があるからだと思います。
ListとArrayList だと、いまいち利点がわかりにくい気もしますが。
業務ロジックの切り替えを動的に行いたいときなんかに重宝します。

昔は、XMLにクラス名書いて、Class.forName() とnewInstance() 使ってインスタンスを作成してたりした記憶があります。
Seasar2 だと、変数名でDIできるので楽ですね。

CMP

間違ってたらごめんなさい。
古いタイプの人(私も含む)だと、「わざわざArrayListで生成しているのであれば、そのロジック内ではArrayList固有の機能を使用しているはずだから、Listに格納することは潜在的な不具合を生み出すもとになっているのではないか?」と考えたのではないでしょうか。
Listに関してはそういうことはないみたいですが、Javaに慣れていない人からすると「ぱっと見」ではそう感じてしまいます。例え変化に強いと言われても、固有の機能を使用した「作り」になっていれば、結局書き換えるわけですから。(で、書き換え忘れが不具合になる・・・はず)
「いや、そんなはずはない!」ってところのコメントがいただけるとありがたいのですが。

P.S.
オブジェクト指向って「(自分には)理解できない=(自分には)使えない」であって、「(自分には)理解できない=(他人にも)使えない」てのは間違いです。そのことが年寄りになればなるほど理解できなくなっています。いい加減、あのおっさんも気がつけよなぁ。

flatline

CMPさん、こんにちは。

>古いタイプの人(私も含む)だと、「わざわざArrayListで生成しているのであれ
>ば、そのロジック内ではArrayList固有の機能を使用しているはずだから、Listに
>格納することは潜在的な不具合を生み出すもとになっているのではないか?」と
>考えたのではないでしょうか

たとえば、List#get(int index) は、指定したindex が範囲外の場合、
IndexOutOfBoundsException を投げること、とJDKのJavaDoc に記述されています。
ある人が、List の具象クラスとして、HogeListクラスを作成したとして、
index が範囲外の場合、例外を投げるのではなく、null を返すようになっていたら、
それは正しい作り方とはいえないのではないでしょうか。

インターフェイスの具象クラスなら、インターフェースのJavaDocに記述されていない「固有の機能」を実装するのは、そもそも論として、実装方法が変なのではないかなと思います。


>いい加減、あのおっさんも気がつけよなぁ。

あの人はたぶん一生気づかないか、気づいても無視する気がする。

Buzzsaw

今までPGを知らない人間(営業とか口だけSE)相手に、OOPを盾にして闘って
まいりましたが(?)、本当の敵は三浦氏のような輩ですね。
1回目は楽しんで読めましたが、2回目から三浦氏に殺意を抱き始めています。
フィクションなら、三浦氏が殺されても平気ですよね?期待しています。
あー怖い怖い。

CMP

flatlineさんへ

回答ありがとうございました。
とてもわかりやすい例でしたが、「あれ?どっかの現場でみたいような・・・」って気分です。もちろん、正しい作りとはいえないやつです。

>インターフェイスの具象クラスなら、インターフェースのJavaDocに記述されてい
>ない「固有の機能」を実装するのは、そもそも論として、実装方法が変なのではな
>いかなと思います。
なるほど。変な実装をしてるインターフェースでない限りは、考えすぎということでいいのですね。

AC/DCさんへ
なにもリンクを張らなくても(汗)。

ぽこ

コラムの三浦さんの言い分に一理あると思っているのは私だけなのでしょうか?
富永さん(と平良さん)は結局、そのコードのその部分をポリモーフィズムを使用して変更に強くする必要があるのかを説明していない(説明出来ていない)です。

三浦さんが突っ込まれた場所は、仕様と実装が分離されてなければならない相応の理由があるのですよね?
なぜ、富永さんはそれを説明しないのか、甚だ疑問です。

富永さんが、何も考えずにそうコーディングしたのであれば、中身のないカッコつけと思われても仕方がないのでは?

#私自身はオブジェクト指向の考え方は役立つものと思っています。

Ted

初めまして。
三浦さんのモデルが(笑)

オブジェクト指向の利点の一つは他の方も書かれたとおり、型と実装の分離ですね。
私も、LinkedListや独自のList実装クラスに置き換える影響を考えてList l = new ArrayList();と書きます。
もっともこの記法にも欠点はあって、ListインタフェースはCloneableインタフェースを知らないのでl.clone();と書けません。
class MyList extends ArrayList implements List, Cloneableというクラスを用意すれば良いだけですが。

もう一つ、処理を実体として分離できるようになったことも利点と思います。
Cで同じ配列を違う整列基準でソートするには、整列関数を関数ポインタで渡しますが、この処理が別の変数を参照するとクロージャの問題が出てきます。
オブジェクト指向言語ではクロージャを素直にオブジェクトとして表現できるのでシンプルだし便利ですね。

続編を楽しみにしております。

Ted

すみません、いきなり間違えました。
必要なのはMyListクラスではなくinterface CloneableList extends List, Cloneableというインタフェースでした(汗)

精廬

はじめまして。

ArrayListをListで持つことには最初違和感を持たずにいました。
しかし、よく考えるとCollectionインターフェースでも別に良いよなぁと思いました。

1) Collection list = new ArrayList();
2) List list = new ArrayList();
3) ArrayList list = new ArrayList();

この質問の後、三浦さんが「じゃあなんでCollectionにしないの?」と聞いたら、ちょっと面白い気がしてきました。
1でも3でもなくなぜ2が「普通」なのか?

Fufuhu

たぶんCollectionなんて三浦氏が知ってるなんて思えないんですが…
LinkedListとかHashMapも知らなさそうですね。
(で、HashMapつかったらHashTableつかいなさいと言ってくるかなww)

むしろCOBOLでなぜ作らない!?と三浦氏には質問したいですね。

Ted

精廬さんへ

Java 6 APIを確認しました。
Listは添字で要素を指定できますがCollectionには添字や要素の位置という概念がないようです。
イテレータや要素の包含チェックだけならCollectionで大丈夫ですね。

TypeCpp

ぽこさんへ

> 富永さん(と平良さん)は結局、そのコードのその部分をポリモーフィズムを使用して変更に強くする必要があるのかを説明していない(説明出来ていない)です。

変更に強くする必要を説明しなければならないという考えが理解できません。
今回のケースでは、コストがかかるわけでなし、デメリットがあるわけでなし、それでいて変更に強くなるというメリットが得られるわけで。
もしかして、ぽこさんは「ソフトウェアは変更に弱くあるべき!」という考えの人ですか!?

Fufuhu

>ぽこさんへ
なんだかうちの犬と同じ名前なのですが、あえて批判をば
あとから手を入れられることのないソフトウェアって
ほぼ”使われていないソフトウェア”だと思うんです。
つまり、使い捨てor出来が悪いソフトウェアってなります。
将来的にも改良しながら使いつづける事を考えるなら変更に強くするのは
ソフトウェア開発の基本だと思うんですけど…

ひとつのシステムの全面的な刷新が短くて5~10年に1回と言われている中で
他は機能の付け足しやバグフィックスが主だとおもうんです。
その条件なら変更に強くないとお話にならないと思います。

少なくとも変更に弱い作りのソースコードは自分には回して欲しくはないです。
単価が高いなら別ですが…

九古

変更に強いソースコードの方が望ましい事には異論ありませんが、
List list = new ArrayList();

ArrayList list = new ArrayList();
と比べて変更に強いかというと、
そんな事はないと思います。

どっちにしてもソースコードに手を加えなければならないのなら、
List list = new LinkedList();
に変えるのも
LinkedList list = new LinkedList();
に変えるのも、違いはありません。
clone()を使う形への変更なら、
逆に
List list = new ArrayList();
の方が一手間多いですし。

私個人に関して言えば、List list = new ArrayList();をいつも使っていますが、
それは、慣れの問題だと思います。
(List listの方をいつも使っているので、ArrayList listが出てくると、
Listではダメなケースがあるのかと反射的に予想してしまい、
据わりが悪いのです。)

Listに関して三浦氏の指摘がイヤなのは、
どっちが良いとも言えないようなチマチマした重箱の隅をつついて、
時間とやる気を削ぎまくっている、という点と、
ポリモーフィズムが有効なシーンを知らないまま、
List list = new ArrayList();
というどっちでも良いような一例を以って、ポリモーフィズム全体を無用とみなしている点であると思います。

ぽこ

>TypeCppさん
三浦さんは、変更が発生する要因が見えてないのです。
平良さんらには見えているのでしょう?
ならば、その要因を説明し、その要因の妥当性を闘わせるのが筋ではありませんか?
闘いの結果がどうなるかは分かりませんが、少なくともチーム内での共通認識が
できます。
私は、この共通認識をつくり上げることがレビューで大切なことと思いますが、
いかがでしょう?
それが、「(平良さんらが)そのコードのその部分を変更に強くする必要を説明しなければならない」と考えている理由です。三浦さんには出来ません。

#ちなみに、このコラムに掲載されている範囲では
#私も変更が発生する要因が分かりません^^;
#よろしければ、TypeCppさんが見つけた要因を教えていただけませんか?

>Fufuhuさん
すみません、私の主張の何処が批判されているのかが分かりません。
私は「ソフトウェアでは変更に強くする必要はない」とは主張していません。
「説明できる人が説明を行っていないのは問題ではないか?」と主張しています。

通りすがり

横から失礼します

>Tedさん
>Listは添字で要素を指定できますがCollectionには添字や要素の位置という概念がないようです。

添字でアクセスする場面では大抵それが O(1) だと期待するでしょうから、
それならば RandomAccess を実装している ArrayList の方が適切だと考えます。

とおりすがり

はじめまして。

> 精廬さんへ
メソッドの外に出ない変数なら
Collection型でも問題ないと思います。
というかArrayListでも問題ない。
この場合は目的を満たせる型なら何でもいいです。

しかしメソッドの戻り値となる変数の場合は
ちょっと考える必要があるかな、と思います。

戻り値の型がCollection型のメソッドは使いにくいです。
ソートするのにListか配列をを作らないといけませんから。

型の抽象度は
●使われる側は高めたい(変更に備えたい)
●使う側からは低くあってほしい(具体的な方が使えるメソッド多くて便利)
というトレードオフがあって、
そうなると配列に近い感覚で扱えるListはバランスがいいのだと思います。

あら?
本筋から外れてますね…

haraheri

はじめまして。

実は私も三浦さんと似たような疑問を持って、
Listの機能しか使わないんなら
  List list = new List();
でイイではないかと思いました。(ListとArrayListが何者かを知るまでは・・・ですが)

「細かく知らないだろうManager(コボラー)」に、Reviewの場で自分が書いたコードの理由を
簡潔に説明出来ないのは、プロジェクトにとって不幸な事に感じますね。
話が変な方向に行く前に、平良さんが「じゃあ富永さん、簡単に説明して」と言うだけで済んだような気もします。
「なんか変ですか?」「一般的です」「みんなやってます」「はい、そうですか」でコードレビューが終わる方が怖いかも。

ある

シリーズが完結していたのでコメントを。
(議論も終わっているので無意味に近いですが)

まず、ぽこさんを批判した人は完全に間違っています。
三浦さんは理由が分かっていません。
そして、平良さんは説明ができていません。

「お言葉ですが、この記述はごく一般的なものだと思います」
「いえ、しかし、仕様を実装から分離するのはオブジェクト指向の基礎ではないでしょうか」
これで説明したということになりますか?
Listを使う理由。たしかにわかっていれば上記の説明だけで通じるかもしれません。
技術者であれば通じるかもしれません。
管理者に同じこと説明しますか?
そもそも、新人にどうやって説明しますか?
説明がこの程度であれば負けて当然。

「ああ、そういう細かい理屈はいいからいいから。とにかくインターフェイスなんか使う必要ないの」
この理論を突破するには新人向けの説明が必要でしょうが、プロマネ系の方であれば、
工数、人件費、運用開始後のリスク等の技術、管理両面から攻めることができるので
金額面含め説明できるので勝てるでしょう。

通りすがり

ちゃんと全部読まずに書き込んじゃいます。

interface の型で宣言するのって慣習みたいな感じだからどうでもいいんじゃないですかね?しつこく詰問して精神的に生産性下げる方が馬鹿馬鹿しいです。
ループ用の変数を i や j で宣言するようなもんじゃないかなーなどと思いました。
「アルファベットは何から始まる?ループ変数って普通は a, b だろ」ってコードレビューで言われると嫌じゃないですかね。

さらにこの部長も平良さんに分かるような説明はしてないし、平良さんにだけ分かるような説明求めるのもフェアじゃない感じがしました。もともとは大したことではないはずの、コーディングスタイルの話題でしかないのに工数やリスクの話題に持って行こうとするのは、結論を都合のいい方向へ持って行くための方便ではないですかね。現場でよく見られます。笑

hegemon

「変更に強くする」では理由付けが弱くなるのは「変更が必要かどうか不明」だからですね。
しかし、「システムをシンプルに保つ」というのは必ず必要になります。なぜなら、あらゆるシステムは要件を満たす最大規模にまで膨らむからです。そして、大規模なシステムをきちんと作り上げるためには、可能な限りシンプルでなければなりません。
ある変数なり戻り値の実装がArrayListかLinkedListかという情報は、ほとんどのケースでは必要でない余分なものです。それよりは抽象度の高いListの方が単純で済みます。

こう考えると、オブジェクト指向のカプセル化もポリモーフィズムも、同じく「いかにシンプルにするか」が目的であると納得いくはず。

ponde

>「アルファベットは何から始まる?ループ変数って普通は a, b だろ」ってコードレビューで言われると嫌じゃないですかね。
そんなアホなこと言われたら
iはindexの頭文字ですが?indexの意味、ご存じですか?


って強く言えるぐらいおれも強くなりたい。。

ン年前のコメントだけど...

> 「説明できる人が説明を行っていないのは問題ではないか?」と主張しています。
問題とする箇所を間違えています。。

デファクトスタンダードがスタンダードである理由を説明する価値はありません。
「スタンダードだから」という理由だけで説明は事足ります。さらに深い話(プログラミング理論とか計算機科学とか)はもはや学問の世界、学者先生の範疇であり、一プログラマが必ずしも知っておくべき知識ではありません。

この場合は、スタンダードに対して異を唱える三浦mgrのほうに、何故ダメなのか何故あえて定石を打たないのか説明する責任があります。


そしてレビューを紛糾させている指摘するのもばかばかしい最大の問題は、Javaの知識がないレビュアーがコードレビューをしている点です。

コメントを投稿する