ページ内ジャンプ:
スラッシュドット・ジャパン 全文検索

アレゲなニュースと雑談サイト

コードレビューって意味あるの ?

reoによる 2009年06月25日 11時30分の掲載
どうなんでしょ部門より。
プログラミング 日本

pinbou 曰く、

本家 /. の記事 ``Are Code Reviews Worth It ?'' より。悩めるマネージャからのご相談。

私は開発マネージャです。先日、上司と私はコードレビューの価値について議論しました。私の部署ではプログラマ主導でコードレビューとデザインレビューをやっています。やってみてわかったことは、コードレビューはやたら時間がかかる割に、良質な UI レベルでのテストよりも分かることが少ないということです。一方、デザインレビューの効用は絶大です。そもそも私たちの書いているコードはデスクトップ向けの、非クリティカルな用途のものなので、私は上司に、コードを精査するのはあまり意味がないのではないかと尋ねてみたのです。Slashdot の皆さんはどう思われますか。

コードレビューって意味あるの ? | ログイン/アカウントの作成 | 89 個のコメント | コメント検索
表示オプション しきい値:

  • もちろん意味がありますとも (スコア:4, すばらしい洞察)

    okky (2487) : 2009年06月25日 17時31分 (#1593853) ホームページ 日記

    「こういうコードが恥ずかしいコードである」
    という価値観について、上級技術者間で意識統一がなされていればね。

    ようするにコードレビューと言うのは、大学の研究室で言う輪講とかと同じなんです。
    コードをよりよいものにする、と言うのも目的の一つですが、コードを組んだ人のレベルアップを図る、という目的もある。

    十分な人数の、良く判っているプログラマがいるならばペアプログラミングも良いでしょう。でもペアを組んで回れるほどレベルの高い人がいなかったら?
    「教授と助教授と助手の目の前で発表させる」
    しかないじゃないですか。

    もちろん、この作業は「教授や助教授や助手」の時間を食います。もしあまりにも多くの時間を食うのであれば可能性は次の３つのどれか。

    1. 初心者が多すぎる。そのため、「教授や助教授や助手」の時間をフルに使っても、全部など到底見切れない。コードの品質は悪いままである。
    2. 初心者が少なすぎる。コードの品質は最初から高く、いくら見ても時間の無駄である
    3. 「教授や助教授や助手」のレベルに到達した技術者が実はいない。なので見当違いな所を見ていたり、全員が同じ間違いを犯していていくらレビューしても品質は向上しない

    UIレベルのテストで問題がたくさん出てくるのですから 2 である可能性は低いですね。

    1 かどうかは、レビューを受ける人とレビューをする人の人口比率を調査する必要があります。
    基本的に初心者と判りきっている人に、全コードレビューに参加させるのは得策ではありません。もちろん、良いコードを読むのは重要ですが、自分が組んでいる部分と全く関係の無いコードの、しかも最初のバージョンのレビューに参加しても得られるものは少ないでしょう。それよりも「直せ」と言われた部分を直す事に時間をかけるべきです。
    逆に全てのレビューに「教授・助教授・助手」レベルの人たちは参加する必要があります。実質的にコードの質を向上させているのはこの人たちですから。この人たちの時間がレビューに取られる、というならば、この人たちはコーディングをするべきではないのです。それでも足りないなら、プロジェクト全体の人数が多すぎる、と言う事です。

    3 … えー、3かどうかは NDA を結んだ外部のプログラマを何人か雇って、彼らのコードレビューを受けるべきだと思います。それまで発見された問題とかなり違う問題点がいくつも発見されるようならば、 3 の状態である可能性は非常に高いです。

    --
    fjの教祖様

    • > 1. 初心者が多すぎる。そのため、「教授や助教授や助手」の時間をフルに使っても、全部など到底見切れない。コードの品質は悪いままである。
      > 2. 初心者が少なすぎる。コードの品質は最初から高く、いくら見ても時間の無駄である
      > 3. 「教授や助教授や助手」のレベルに到達した技術者が実はいない。なので見当違いな所を見ていたり、全員が同じ間違いを犯していていくらレビューしても品質は向上しない

      結論：ほとんどの日本のIT企業においては、コードレビューは時間の無駄である。

      通常は１＆３。運が良ければ１だけのこともある。

  • あくまで道具のひとつ (スコア:3, 興味深い)

    Anonymous Coward : 2009年06月25日 11時45分 (#1593530)
    個人的な位置付けとしては単なる一つの手法。
    そして、コードレビュー単体として用いるのはあまり効果が無くて、
    開発手法の中のその他の手法と組み合わせることで有用なこともある、といった感じ。
    ただし時間が結構要るのは否めなく、その時間を取れないことが流行らない理由だとも思う。

    まあコードレビューというより、コード座談会みたいなのが現代にマッチする手法かもね。
    その時点でのコード・手法を皆でざっくばらんに話し合い、良い点悪い点、悩みどころなどを
    気軽にぶつけあえる場を持てるような環境。どうしても個に閉じてそれっきり、ってな
    雰囲気で進んでしまい、それだと進歩や改善というのが得られ難いから。

  • 全ては今後の保守のために (スコア:3, 興味深い)

    SorabeAlto (35110) : 2009年06月25日 20時47分 (#1593954)
    命名編：
    「このgetData()って何してるの？え、データの取得？データの何？HogeHoge？
    だったらgetHogeHoge()って名づけようよ。んでこっちのgetData2()ってのは？」

    手段編：
    「転送する文字列作るのにいろいろ苦労してるみたいだね。分割したり連結したり
    数値変換して、malloc()してrealloc()で伸張してまたmalloc()して（あ、メモリリーク）
    ……ねぇ、これってsprintf()で済むんじゃない？え、sprintf()知らない？
    君、専門学校でC言語習ったって言ってたよね？」

    効率編：
    「このforループの中でiniファイルから値取る関数呼んでるけど、
    仮に1,000回ループしたら、同じ値を取得するファイルアクセス処理を
    1,000回実行することになるよ。え、サーバで動かすから速い？
    そういう問題じゃねぇ！forループの直前で呼べば1回で済むだろうが！」

    今日もまた、仕事のヤリ逃げを阻止する仕事が始まるお
    --
    ----------宙辺 或人

  • 回路は他人が設計してもある程度分かりますが、
    他人の作ったコードは本当に理解できないと思う。

    回路でもそれぞれの好みや設計思想があるけど、
    コードは変数の名前の付け方一つで全く理解できなくなってしまう。

  • 身も蓋もない話 (スコア:2, すばらしい洞察)

    Anonymous Coward : 2009年06月25日 12時04分 (#1593546)

    世の中には意味のあるコードレビューと、意味のないコードレビューがあって
    それぞれを行っている人達が議論しても絶対にかみ合わない。

  • コードレビューの効果 (スコア:2, おもしろおかしい)

    Anonymous Coward : 2009年06月26日 0時44分 (#1594086)
    当社ではコードレビューを導入してから、いろいろなメリットがありました！
    • 研修も兼ねて新人の女性プログラマーをレビュー担当にしたところ、プログラマーが美しくてわかりやすいコードを率先して書くようになりました。
    • 社外のIPアドレスからシステムにアクセスすると、何故かウェブUIにアフィリエイトバナーが表示されるような不可解な不具合が激減しました。
    • プログラム中の無駄な空白行やコメントが減り、生産性が向上しました。
    • ソースコードから、クライアントや上司への悪口が無くなりました。
    • プログラマーがレビュー会議中に睡眠時間を確保できるようになりました。
  • 何人かの方が書いているように、コードレビューは全体のレベルアップ
    に非常に役立ちます。

    もし、コード品質があまりよくないのにコードレビューが役に立っていない
    と感じるならば、外部からレビュアーをつれてくるのが大変役立つと思います。
    その「外部からのレビュアー」を誰がどう選ぶのか、という問題はありますが。

    が、みんなが優秀なら不要、という趣旨の論にはちょっと違和感あります。

    商業的な文章は、たとえどんなプロが書いたものであってもかならず
    編集者のチェックがはいりますよね。それと同じようなもので、商業的
    なコードはコードのチェックをすべきで、その一手段としてコード
    レビューがあると思います。なんならペアプログラミングでもよいです。

  • 意味はあります。 (スコア:1, すばらしい洞察)

    Anonymous Coward : 2009年06月25日 11時47分 (#1593531)

    あなたが気にしてるのは、その意味を得ることが、その作業を行うためのコストに見合う価値を
    持っているかって話でしょう？そんなのは状況次第で、状況設定なしに回答できるもんじゃない。

    　とりあえず言えることは、「コードレビューをやらなかったらどうなるのか」を試すとき、
    コードレビューを日常的に受けてきてる人らのコードに対してそれをやってもあまり意味がない
    ことには最低限気づくべき、ということくらいです。

  • 程度による (スコア:1)

    backyarD (36899) : 2009年06月25日 12時30分 (#1593575) 日記

    VB.NETで文字数をカウントする処理を書かせたら、

    1.配列に1文字ずつ文字を切り出して入れて
    2.配列の内容をFor文で1文字ずつ読み出してループ回数を取得
    3.ループ回数を結果として返す

    みたいなのを書いてくるような組織（実話）に対しては、最初にガツンと釘を刺すという意味で有効だとおもいました。
    あぁ、あと、同じくVB.NETでフラグ的な位置づけの変数をStringで宣言して「"True"」とか突っ込むようなチームとか。

    ＃いずれも実話

  • 拡張子をチェックするのに、
    　　if(strExt == "abc" || strExt == "ABC")
    という風に書いてあるコードを見た。
    "Abc" は来ないと思ってるのか～?!

    new や alloc の後に、メモリが確保できたかどうかチェックしてなかったり、
    try catch に挟まってたらいいけど、そうではなく..
    開発PCからメモリ抜くぞ!

  • プログラマのによりソースコードの品質がばらつくなら、
    コードレビューはいいかもしれません。
    それより、ペアプログラミングの方がその目的を達成しやすい様な気がします。

    レビューまで待たずにその場で修正させた方が良いですからね。
    しかも、その場で問題点と解決方法が出てくるのでより効果的だと思います。

  • 書き方のルールがかっちり決まってて、それに合わせなくちゃというモラルもあるなら、ミスが発生しやすいポイントなどのノウハウの蓄積にはなるのではないかと。

    --
    ◆IZUMI162i6 [mailto]
    Free or not Free, that is the question.

  • ＞オプソとかを流行らせないでくれ。
    ＞OSSって書けばもっと短いし

    短くなってないやん。

  • 5個のコメント が現在のしきい値以下です。

このページのすべての商標と著作権はそれぞれの所有者が有します。 コメントやユーザ日記に関しては投稿者が有します。
のこりのものは、© 2001-2010 OSDN です。