🚓

コードレビューでプロダクトオーナーが見る観点について

2024/12/16に公開

自己紹介

はじめまして。とあるスタートアップでプロダクトオーナー(PO)をしています。それまでは約4年ほどRuby on RailsやReactを中心にWebアプリケーション開発に携わってきました。

概要

本記事では、プロダクトオーナーがコードレビューに参加する際に、どのような観点からコードをチェックしているのかを紹介します。一般的にコードレビューはエンジニアが行うイメージが強いですが、プロダクトの方向性やビジネス上の要件を決める立場であるPOが参加することで、仕様との整合性やドメイン理解の深さなど、違った角度からのフィードバックが可能です。

本記事で紹介する観点は以下の通りです。

  1. 仕様通り実装できているか
  2. 実際のドメインと異なった実装になっていないか
  3. テストケースが十分に網羅されているか

1. 仕様通り実装できているか

プロダクトオーナーとして一番重視するポイントは「仕様に沿った実装がなされているか」です。事前に合意した要件やデザインなどと実際の機能挙動を照らし合わせて確認します。

  • 挙動確認: ローカル環境やステージング環境で実際に機能を触ってみる。
  • コードでの確認: 挙動に疑問がある場合、関連するコントローラーやモデル、Reactコンポーネントの処理を追ってみて、条件分岐やメソッド命名が仕様に反していないかを確認。

Railsのコントローラー例

# 仮の仕様:ユーザーがプロフィールを更新する際、"bio"フィールドは255文字以内であること。
# コード例を見て仕様が正しく実装されているか確認する。

class UsersController < ApplicationController
  def update
    @user = User.find(params[:id])
    if @user.update(user_params)
      render json: { message: 'Profile updated successfully.' }, status: :ok
    else
      render json: { errors: @user.errors.full_messages }, status: :unprocessable_entity
    end
  end

  private

  def user_params
    params.require(:user).permit(:name, :bio)
  end
end

# ここで、Userモデル側にバリデーションがあるかを確認する
# Userモデル
class User < ApplicationRecord
  validates :bio, length: { maximum: 255 }, allow_nil: true
end

このようにコードを確認することで、仕様で要求された制約が実際にモデルレベルで担保されているかを確認します。「最大文字数が255文字まで」という仕様が正しく実装されていることがコード上で確認できます。

2. 実際のドメインと異なった実装になっていないか

仕様には合っていても、ドメイン的な意味合いが崩れている場合があります。例えば、「利用者」(User)と「スタッフ」(Staff)をドメイン上は明確に区別しているのに、実装上は同じモデルを無理やり流用していると、後々ドメイン理解が曖昧になり、機能拡張で混乱を招きます。

  • ドメインモデルの整合性: ネーミングやクラスの責務がドメイン理解に合っているかを見る。
  • 命名・責務の一貫性: Userモデルが実際には"Staff"として機能していないか、あるいは本来"Order"で表されるべき概念が"Reservation"など曖昧なネーミングになっていないかを確認。

ドメインの例

# ドメイン上、EmployeeはStaffと異なる役割を持ち、Employeeには給与計算といった特別なロジックが必要。
# しかし、コード上では単にUserモデルでRoleを指定しているだけ、というケースを想定。

class User < ApplicationRecord
  # role: "employee", "customer"
  # 実際のドメインではEmployeeとCustomerは大きく異なる概念だが、同一テーブルに詰め込んでいる
  # もしドメイン上、employee向けに給与計算などのロジックが後から必要なら、StaffやEmployeeモデルとして分けるべきか検討すべき。
end

こういった場合、POとしては「Employeeは顧客(Customer)とまったく別の存在で、今後は給与計算や勤怠管理が必要になる。この状態だとドメイン上の区別が曖昧では?」といったフィードバックを行います。修正が難しければ、そのコード上にコメントを残して、後にドメインモデルをリファクタリングできるような注釈を促すこともあります。

3. テストケースが十分に網羅されているか

仕様やドメイン理解を担保する上でテストコードは重要な要素です。手動での挙動確認だけでは漏れが生じやすく、また新機能追加やリファクタリングで既存機能が壊れないかを検証するためにもテストは不可欠です。POとしては、仕様上の重要ケースやエッジケースがテストコードでカバーされているかを確認します。

  • 仕様に基づいたテスト: 正常系はもちろん、入力値が最大文字数を超える場合、アクセス権限がないユーザーが操作する場合など、仕様で定めたエッジケースがテストされているかを確認。
  • 網羅性の確保: エンジニアの判断でカバー率が十分なケースでも、ビジネス上のクリティカルポイント(支払い処理、アカウント凍結など)はより注意深くテストが書かれているかを見る。

RSpecでのテスト例

# 仕様: bioは最大255文字まで
# このテストが仕様を反映できているかを確認。
require 'rails_helper'

RSpec.describe User, type: :model do
  describe 'validations' do
    it 'is valid with a bio of 255 characters' do
      user = User.new(name: 'Alice', bio: 'a' * 255)
      expect(user).to be_valid
    end

    it 'is invalid with a bio longer than 255 characters' do
      user = User.new(name: 'Bob', bio: 'a' * 256)
      expect(user).not_to be_valid
      expect(user.errors[:bio]).to include('is too long (maximum is 255 characters)')
    end
  end
end

このテストを確認することで、POとしては「最大文字数を超えた場合、エラーが起きる」仕様がきちんと自動化されていることを確認できます。実際に255文字を超える文字列をフロントエンドから入力する手動テストを行うのは大変ですが、テストコードで保障されていれば、安心して将来的な機能拡張やリファクタリングが可能です。

まとめ

プロダクトオーナーがコードレビューに参加するメリットは、以下のような点にあります。

  1. 仕様の担保: 実装が合意済みの仕様どおりであるか、機能が要件通りに動くかを確認。
  2. ドメイン理解の反映: コードにビジネスドメインの意味が正しく反映されているかを担保。
  3. 品質の底上げ(テスト網羅): テストコードを確認することで、ビジネス上重要なポイントがしっかりカバーされているかを確認。

エンジニアリング観点からのコードレビューはコード品質やパフォーマンス、可読性、アーキテクチャ的な整合性を重点的に見ますが、POの視点から加わることで、プロダクトの方向性やビジネス価値に対してのコードの合致具合が強化されます。結果として、チーム全体でより強固なプロダクトを素早く作り上げることが可能となります。

Discussion