レビューチェックリスト

// レビュー対象のコード
function calculateDiscount(price: number, discountRate: number): number {
  return price * (1 - discountRate);
}

// レビューコメント:
// ⚠️ [Must Fix] エッジケースが考慮されていません
// - price が負の値の場合はどうなりますか？
// - discountRate が 0〜1 の範囲外の場合は？
// - discountRate が 1 を超えるとマイナスの価格になります

// 改善案
function calculateDiscount(price: number, discountRate: number): number {
  if (price < 0) {
    throw new Error('Price must be non-negative');
  }
  if (discountRate < 0 || discountRate > 1) {
    throw new Error('Discount rate must be between 0 and 1');
  }
  return Math.round(price * (1 - discountRate));
}

// レビュー対象のコード
class UserService {
  async createUser(data: CreateUserDto) {
    // バリデーション
    if (!data.email.includes('@')) throw new Error('Invalid email');
    if (data.password.length < 8) throw new Error('Password too short');

    // DBに保存
    const user = await this.db.insert('users', data);

    // メール送信
    const transporter = nodemailer.createTransport({ /* ... */ });
    await transporter.sendMail({
      to: data.email,
      subject: 'Welcome!',
      html: '<h1>Welcome to our service!</h1>',
    });

    // ログ出力
    console.log(`User created: ${user.id}`);

    return user;
  }
}

// レビューコメント:
// ⚠️ [Should Fix] UserService がバリデーション、DB操作、
// メール送信、ログ出力の全てを担当しています。
// 単一責任の原則に基づいて分離することを提案します。

// 改善案
class UserService {
  constructor(
    private readonly userRepository: UserRepository,
    private readonly emailService: EmailService,
    private readonly logger: Logger,
  ) {}

  async createUser(data: CreateUserDto): Promise<User> {
    const email = Email.create(data.email);       // Value Object でバリデーション
    const password = Password.create(data.password);

    const user = await this.userRepository.save(
      User.create({ email, password })
    );

    await this.emailService.sendWelcome(user.email);
    this.logger.info(`User created: ${user.id}`);

    return user;
  }
}

// レビュー対象のコード
function proc(d: any[]) {
  const r: any[] = [];
  for (let i = 0; i < d.length; i++) {
    if (d[i].s === 1) {
      if (d[i].a > 18) {
        if (d[i].t === 'premium') {
          r.push(d[i]);
        }
      }
    }
  }
  return r;
}

// レビューコメント:
// ⚠️ [Should Fix] 命名とネストの改善を提案します
// - 関数名・変数名が省略されすぎて意味が不明です
// - ネストが4階層と深く、読みにくくなっています
// - any 型は避けてください

// 改善案
interface User {
  status: number;
  age: number;
  tier: string;
}

function filterActivePremiumAdults(users: User[]): User[] {
  return users.filter(user =>
    user.status === UserStatus.ACTIVE &&
    user.age > MINIMUM_ADULT_AGE &&
    user.tier === 'premium'
  );
}

const UserStatus = { ACTIVE: 1, INACTIVE: 0 } as const;
const MINIMUM_ADULT_AGE = 18;

// レビュー対象のコード
async function findUser(name: string) {
  const query = `SELECT * FROM users WHERE name = '${name}'`;
  return await db.execute(query);
}

// レビューコメント:
// 🚨 [Must Fix] SQLインジェクションの脆弱性があります
// ユーザー入力を直接クエリに結合しています。
// name に "'; DROP TABLE users; --" が渡されると
// テーブルが削除されます。

// 改善案: パラメータ化クエリを使用
async function findUser(name: string) {
  const query = 'SELECT * FROM users WHERE name = $1';
  return await db.execute(query, [name]);
}

// さらに良い: ORMを使用
async function findUser(name: string) {
  return await userRepository.findOne({ where: { name } });
}

// レビュー対象のコード
async function getOrdersWithProducts() {
  const orders = await orderRepository.find();           // 1回のクエリ
  for (const order of orders) {
    order.products = await productRepository.findByOrderId(order.id);  // N回のクエリ
  }
  return orders;
}
// → 注文が100件あれば、101回のDBクエリが発生する

// レビューコメント:
// ⚠️ [Should Fix] N+1クエリ問題が発生しています。
// 注文数に比例してクエリ数が増加し、パフォーマンスが劣化します。

// 改善案: JOINまたはEager Loading を使用
async function getOrdersWithProducts() {
  return await orderRepository.find({
    relations: ['products'],  // Eager Loading で1回のクエリに
  });
}

// レビュー対象のテスト
describe('calculateDiscount', () => {
  it('should calculate discount', () => {
    expect(calculateDiscount(100, 0.1)).toBe(90);
  });
});

// レビューコメント:
// ⚠️ [Should Fix] テストケースが不足しています。
// 以下の追加をお願いします:
// - 割引率 0% のケース
// - 割引率 100% のケース
// - 境界値（price = 0）
// - 異常値（負の価格、範囲外の割引率）

// 改善案
describe('calculateDiscount', () => {
  it('should apply 10% discount', () => {
    expect(calculateDiscount(100, 0.1)).toBe(90);
  });

  it('should return original price with 0% discount', () => {
    expect(calculateDiscount(100, 0)).toBe(100);
  });

  it('should return 0 with 100% discount', () => {
    expect(calculateDiscount(100, 1)).toBe(0);
  });

  it('should handle zero price', () => {
    expect(calculateDiscount(0, 0.5)).toBe(0);
  });

  it('should throw for negative price', () => {
    expect(() => calculateDiscount(-100, 0.1)).toThrow();
  });

  it('should throw for invalid discount rate', () => {
    expect(() => calculateDiscount(100, 1.5)).toThrow();
    expect(() => calculateDiscount(100, -0.1)).toThrow();
  });
});