質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

90.53%

  • データベース設計

    145questions

    データベース設計はデータベースの論理的や物理的な部分を特定する工程です。

  • アーキテクチャ

    81questions

    アーキテクチャとは、情報システム(ハードウェア、OS、アプリケーション、ネットワーク等)の設計方法、設計思想、設計思想に基づいて構築されたシステム構造をアーキテクチャと呼びます

  • デザインパターン

    62questions

    デザインパターンは、ソフトウェアのデザインでよく起きる問題に対して、解決策をノウハウとして蓄積し再利用出来るようにした設計パターンを指します。

  • コードレビュー

    41questions

    コードレビューは、ソフトウェア開発の一工程で、 ソースコードの検査を行い、開発工程で見過ごされた誤りを検出する事で、 ソフトウェア品質を高めるためのものです。

  • ドメイン駆動設計

    31questions

    ドメイン駆動設計(Domain-driven design, DDD)とは、ソフトウェアの設計手法、および設計思想や哲学のことです。ドメインモデル構築の際に、設計上の判断を決定する枠組みとドメイン設計に関して議論するボキャブラリを提供するものです。

レビューに関して(設計レビュー~ソースレビュー)

受付中

回答 5

投稿

  • 評価
  • クリップ 6
  • VIEW 991

ShintaroIshida

score 70

 設計レビューやソースレビューの意義や効率的な進め方に関して

背景として、属人的なプログラム実装や設計をフラットにしてレガシーな環境をレビューを行って何とか減らしていきたいという思いがあります。gitやチャットを導入して、WEBを介してある程度の情報量は共有出来ていると思っているのですが、チームで共有出来ているかというと感覚としては浸透していない印象です。

今まであまりレビュー文化がないプロジェクトばかりで知見や経験値不足の為質問させて頂きます。
何点か聞きたい事、意見を募りたい事があるので箇条書きにします。
主観やご自身の経験に即した意見でもぜひ参考にしたいので屈託のないご意見頂ければ大変うれしいです。

 レビュー実施の意義

本来レビューとはより広域な視点や経験を持っている方からフィードバックをもらう為に実施するものだと勝手に思っています。少なからず内部的な設計は一番自分が網羅していると仮定して、メンバーにレビューを実施する意義とは何なのでしょうか?
仮に設計した内容の周知等が目的であるのであれば、作った資料やソースを確認頂いて疑問点等あれば個別でフィードバックしてもらうのが効率的だと思うのですが、一同に介し個人のレベルがバラバラの状態でレビューの時間を割く意義はあるのでしょうか?

 具体的なレビューの進め方

一番苦労している所です。
ソースレビューに関しては、ソースを追って説明をしています。(ここの階層にパスを作って新たにこういったクラス群をまとめて格納しています云々。そのクラス・メソッドはこういう処理をしています云々)
この進め方に関しては、上述のようにコミット内容をプログラマ個々人が確認する様にすればOKなのではないか?というレベルです。設計レビューに関しても同様で作成した仕様書を説明して流すという時間をメンバーが一同に介して行っています。
設計レビューに関しては、事前に公開して回覧レビュー等を行ってはいますが非効率に思えてなりません。
成果物を公開してその説明をいちいち追いながらやるというやり方が正解なのでしょうか?

 効率的・効果的なレビューに関する情報

自分でもWEB・書籍等探してはいるのですがやはり選定が難しいと感じています。
参考にした書籍や情報等あれば、是非教えて頂きたいです。

コードレビューに関わらず、領域が広いフローのレビューを対象にしたかったのでタグは関連しそうなものを追加させて頂きました。上記項目に関わらず、何かレビューを実施する・文化を定着させることで意見や感想等あれば(そもそもレビューをやめて、こういった手法をとればいい等)ぜひ参考にさせて頂きたいです。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 5

+1

  • 実装が設計と乖離していないか確認する
  • ソースの実装が規約に従っているか確認する

のどちらに重きを置くつもりでしょうか。

ソースを順に追って、構造を説明する必要は薄いと思います。それは本来(詳細な)設計書で書かれるべきことだからです。
ですから、設計書の通りになっているか、もし変えているならなぜそうしたのか、それはいつ設計書にフィードバックされるのか、というのをレビューするなら分かりますが。
※設計者が知らずに、実装者の都合で設計変えてたら後々大問題になりかねません

設計書を元にスクラッチしている時期ではコードレビューの必要性は薄く(初期の段階で、コーダー全員に規約を認識させるためにやるのはありですが)、むしろテストフェーズ以後、バグ修正におけるコードレビューを手厚くした方がよいかな、とは個人的には思います。
バグ修正では修正したことによる他への影響というのを考慮しなくてはならず、それをコードレビューの段階できちんと潰しておく(むろん設計にもフィードバックするわけですが)のです。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/04/11 10:04

    回答ありがとうございます。

    > ・実装が設計と乖離していないか確認する
    > ・ソースの実装が規約に従っているか確認する

    どちら重視すべきものだと思いますが現時点では、実装・設計の乖離を主にチェックするようになると思います。(コーディング規約もあるのですが、まだまだブラッシュアップ出来ていないので)
    ただ、内部設計段階で先行着手する際にもっと設計思想を共有して共通クラス等を上手く共有出来る為には、レビューが有効かと思ったのもあります。

    ご回答頂いた中で質問があるのですが宜しいでしょうか?(主題とは少しそれるので教えて頂ければで結構です)

    > それは本来(詳細な)設計書で書かれるべきことだからです。
    とあり、回答文の中に設計書という言葉が散見されるのですが、外部設計(画面フローやUI設計です)
    以降のフェーズ(ウォーターフール的な考え方ですが。。)ではどのような設計書を作成されていますか?

    自分は、プログラムフェーズに関してはコメントを拡充させてphpdoc等で自動生成されるものを設計書にしており他には特にそういった設計書を作成していません。(ER図等はありますが)
    もしもそういった資料の他に必要と思われる、また作成されている設計書等があれば教えて頂けないでしょうか?(そもそもそういった物の準備が足りていないのが原因かもと思いまして。。。)

    キャンセル

  • 2017/04/11 10:24

    最近だと主に UML のシーケンス図で処理を記述しますかね。
    ※ですからソースレビューは、「UMLと乖離していないか」が主要な着目点になります
    もう少し前だと「フローチャートを文章で書き記す」レベルの詳細設計を書いていたこともあります。
    むろん、phpdoc や doxygen などのドキュメント自動生成ツールは大いに使うべきなのですが(ライブラリ系だとむしろそちらの方が重要になりますし)、手順というかアルゴリズムは図の形で起こしておかないと後々困ることが多いですね。

    スパイラルモデルやアジャイルだとそういうのを作ってる時間的余裕がない(どころか、アジャイルだと文書生成量が他と比べて格段に少なくなり、ソースこそ全て、という考え方に思える)ので、よほど個々の単位が小さくないと破綻するような気はしています。

    キャンセル

+1

実際に規模の大きなプロジェクトの経験者に聞くのが一番よいと思われます。

コードレビューの意義は様々あると思いますが、やはり使っている技術に一貫性がないと
例えばJavaでファイルの操作でFileクラスとTERASOLUNAフレームワークを違った箇所で使っていると統一性がなかったり、
Javaのバージョンによってラムダ式を使っている箇所もあればそうでないところがあると統一性はなくなります。

無駄な処理やプロパティが残っていたり、コード規約違反があったりと書いた人では気づかない箇所も多少あると思いますし、そういうものを極力なくしていくのは良いことだと思います。

gitを利用して共有できているのであれば、ブランチを
「正式本番」「リリース用」「テスト環境用」「開発用」などに分けて、
テスト環境用以上にマージする際はソースコードを書いた人以外がレビューを行って、
指摘事項がなおった場合、レビュー者がマージしていくというルールを作れば自然とレビューは必要になるはずです。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/04/11 10:12

    回答ありがとうございます。

    > 実際に規模の大きなプロジェクトの経験者に聞くのが一番よいと思われます。
    そうですね。5年以上前ですが一応100人規模のPJに参画していた時期もあり古い情報ですが、その時のマネージメントを一部参考にしながら今試行錯誤しています。そのPJでは、レビューリクエスト(当時SVNでソース管理していたので)をレビュアーに送ってレビュー後マージというまさに仰られる状態で運用されていました。逆にレビュアーの負担が大きすぎるなと思っておりその辺も試行錯誤中です。

    git 運用に関しては、ブランチ・環境の切り分け運用は行っているので問題ないのですがレビューをすると言う文化そのものが定着していないのがやはり問題でしょうね。

    > やはり使っている技術に一貫性がないと
    ここは、自分も痛感している部分なのでやはりレビューとともに規約をブラッシュアップする必要がありますよね。。。

    キャンセル

+1

いままでの経験でいくとチームの構成メンバーによって変わるとおもいます。

レビュー実施の意義

実装前の観点決めや実装中の相談など作業中であれば意義は多分にあります。
できたものをこうだああだと言われるより良いのと、最低限全体共有出来ていれば
誰でもドキュメントを纏められるようになるので情報共有もしやすくなるはずです。
ただ配慮がない人がいるとジワジワと疲弊していくので人員次第とも言えます。

具体的なレビューの進め方

先に何をレビューするかの観点を決めた方がいいです。コーディング規約やロジックの正当性、例外処理が適切か、性能に問題がないか(環境依存があるのでここが1番難しいですね)
これを決めないレビューごっこだと個人の好き好きや宗教観に依存してイチャモン合戦になっているのをたまに見受けています。

効率的・効果的なレビューに関する情報

時間を決めてやった方がいいです。午後1やティータイムなど個々人が決まった時間に見るようにするとレビュー後の作業時間を取りやすいのでタスクが整理しやすくなるはずです。
また対面レビューは実施コストが高いため超重要箇所のみとした方がいいです。
いまの現場でこれを毎回やっている人がいるのですが人員がもろに疲弊してしまうのでオススメしないです。
会社の取り組みに該当してしまうので意外と情報が出てきにくいのですが、企業の開発ブログなどを漁ってみると適当な情報が得られることがあります。

基本的にはチームとしての成熟度に比例するので適切なコミニュケーションが取れていたり、
自立した人が多いチームであればレビュー実施の意義はそれなりに大きいと言えます。
逆に外注さんが多かったり、上の方の人間が機能していないなどチームとしての機能が薄い場合はやり方を塾考した方がいいです。気安く初めて要らぬ揉め事や人的疲弊の温床になるのを度々見かけています。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/04/11 11:56

    イチャモン合戦を避けるべきなのは同意しますが、ほかの部分が私と正反対の点が興味深く感じました。私もレビューがうまく機能しなかった経験も多いので、心にとめておきたいと思います。

    キャンセル

  • 2017/04/11 12:37

    基本的にはする方もされる方も謙虚たれという基本が守られているだけで良いはずだと考えていたのですが、レビューを行う方が奢ってくると悪い影響が波及することが最近分かったので組織内の人員によって細かく決めて行った方が良いなという結論でした!

    キャンセル

  • 2017/04/11 12:52

    足を引っ張るにはバグの報告と共に最適な場所ですよね。マウンティングが激しい上司が参加するレビューは納期ギリギリにするなど、これまたやばい対応をしたことを思い出しました。それを考えると、レビュー(ウォークスルー)とコミットは別フェーズにした方がいいですね。レビュー指摘をもとに、リジェクトされるのは当然ですが。

    キャンセル

0

直接の回答ではありません。

以前、どこぞの大手SIerのOracle RDBMSのSQLのコーディング規約に
・FROM句にはテーブル、ビューのいずれか1つしか記述してはならない。(NoSQLではないのだが)
・SELECTの並びには * 以外を記述してはならない。
ってあったのを思い出す。
Solaris SPARC だったが、パフォーマンスが出るはずはない。爆

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/04/11 10:14 編集

    回答ありがとうございます

    どういった理由からそういう規約を作成したのでしょうねww
    規約を作ったら作ったで、妥当性評価や整備をしないといけないという、いい勉強になりました。

    キャンセル

  • 2017/04/11 11:22

    現場監督を主な仕事にしているSIerさんに多いですが、SQL入門程度の研修すら受けずに汎用機のスキルだけで書いて、その誤りを指摘すると烈火のごとく怒って言われたことだけやっていろ。
    それで作られたシステムを納品されたユーザーさんは遅くて使い物にならないので納品を拒否して。。。修羅場が続きます。

    キャンセル

0

レビューの中でも、ウォークスルーのことをおっしゃっているのだと思いました。

私はウォークスルーは”一人では見逃すことも、大人数では発見できる”という考え方が基本にあると考えています。そのため、全員がわかっているつもりになっていることも一つずつ言葉にして説明し、矛盾を洗いだしたり、漏れを発見したりすることです。

変更点の周知などは、目的外の効用だと思います。

レビューを受ける者は、有効な指摘を受けることを目的に正直かつ丁寧に説明する必要があり、
レビュアーは成果物に集中し、素直で忌憚のない質問と指摘を心がける必要があると思います。

この点においてレビュアーは必ずしも該当システムに詳しい必要はないと思います。テストに詳しいや場合によっては、理由はわからないが質問が鋭いなどの貢献すべき何かを持っていればよいと思います。

逆に、”よくわからないけれども上長または先輩として一言いう必要がある”といった人がある場合、ウォークスルーはやめておいた方がいいかもしれません。

一番詳しい人がコミッタをするからウォークスルーは無しというのも一つの考え方でしょうし、三人寄れば文殊の知恵とウォークスルーで手間をかけるのもありだと思います。

また、ウォークスルーは密なコミュニケーションが取れるので、チームの文化を醸成するのにも有効です。ただし、あくまで副次的なものとして考えた方がよいとは思います。


ググったら出てきた。
http://www.itmedia.co.jp/im/articles/1111/07/news162.html
・資料は事前配布
・指摘の対応は任意で、その場での回答は不要
・管理者の参加は禁止
など、重要な指摘だと思います。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/04/11 15:29 編集

    回答ありがとうございます。

    > ウォークスルーのことをおっしゃっているのだと思いました。
    キーワードを初めて知りました。まだまだ勉強不足ですね。ありがとうございます。

    >私はウォークスルーは”一人では見逃すことも、大人数では発見できる”
    テスト関連は、クロスチェックの兼ね合いもあり比較的人員(工数)を割いてスケジューリングされている認識なのですが、ソース(設計)レビューはまだまだ機能していない事もあり工数確保が難しい現状なのですがこういった観点は説明が行いやすいですね。

    > 逆に、”よくわからないけれども上長または先輩として一言いう必要がある”といった人がある場合、
    > ウォークスルーはやめておいた方がいいかもしれません。
    いるかもしれません。また、意見を述べるからには具体的な解決策も一緒に出せる人じゃないと
    レビューの意義が薄れるし、参加モチベーションが下がりますよね。気を付けたいです。

    > また、ウォークスルーは密なコミュニケーションが取れるので、
    > チームの文化を醸成するのにも有効です。
    > ただし、あくまで副次的なものとして考えた方がよいとは思います。
    最近、チームというキーワードを意識して使うようにしているのですが、この文化形成は今回の質問とは別で自身の課題にありました。副次的であれそういう効果が出るようなレビューというのは難しいのでしょうね。(仲良しごっこも意味ないでしょうし、ギスギスもだめでしょうし)

    ウォークスール 今から勉強してみます。参考URLの提示ありがとうございました。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 90.53%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る

  • データベース設計

    145questions

    データベース設計はデータベースの論理的や物理的な部分を特定する工程です。

  • アーキテクチャ

    81questions

    アーキテクチャとは、情報システム(ハードウェア、OS、アプリケーション、ネットワーク等)の設計方法、設計思想、設計思想に基づいて構築されたシステム構造をアーキテクチャと呼びます

  • デザインパターン

    62questions

    デザインパターンは、ソフトウェアのデザインでよく起きる問題に対して、解決策をノウハウとして蓄積し再利用出来るようにした設計パターンを指します。

  • コードレビュー

    41questions

    コードレビューは、ソフトウェア開発の一工程で、 ソースコードの検査を行い、開発工程で見過ごされた誤りを検出する事で、 ソフトウェア品質を高めるためのものです。

  • ドメイン駆動設計

    31questions

    ドメイン駆動設計(Domain-driven design, DDD)とは、ソフトウェアの設計手法、および設計思想や哲学のことです。ドメインモデル構築の際に、設計上の判断を決定する枠組みとドメイン設計に関して議論するボキャブラリを提供するものです。