Devin.KR

AI 가 만든 PR 리뷰하기 - 그럴듯한 오류 찾기

개발자KR 조회 5

이 장에서 배우는 것

컴파일이 되고 테스트가 통과한 변경에도 주문 금액이나 재고를 잘못 처리하는 코드가 들어갈 수 있다. 특히 AI가 만든 코드는 설명과 변수 이름이 자연스러워서, 읽는 사람이 동작까지 맞다고 받아들이기 쉽다. 리뷰의 출발점은 코드가 얼마나 매끄러운지가 아니라 요구를 어떤 입력에서 지키고 어떤 입력에서 어기는지다.

앞 장에서 테스트 자체를 의심했다면, 이번에는 변경 비교(diff)와 실행 결과를 함께 읽는다. 작은 쇼핑몰 주문 서비스에서 정상 주문, 마지막 재고, 쿠폰 누락, 결제 실패, 금액 반올림, 재고 경쟁을 같은 입력 목록으로 확인한다. 원본과 AI 수정본의 차이를 찾고, 고친 구현도 그 목록에 다시 대조한다.

  • 요구를 입력과 기대 결과로 바꾸어 변경과 대조한다.
  • 경계 조건, null, 예외 처리, 반올림에서 숨은 동작 변경을 찾는다.
  • 동시성 문제를 실행 순서로 풀어 결정적으로 재현한다.
  • 원본·AI 수정본·수정 완료본을 같은 입력으로 실행하고 리뷰 근거를 남긴다.

문제 상황

팀의 주문 서비스에는 수량 검사, 쿠폰 계산, 결제 모의 객체 호출이 한 메서드에 모여 있다. 팀원은 AI에게 읽기 쉽게 정리해 달라고 요청했다. 요구는 기존 동작을 유지하는 것이다. 수량과 재고가 같으면 주문할 수 있고, 쿠폰이 null이면 할인이 없다. HALF 쿠폰은 합계의 절반을 할인하며, 최종 금액은 소수 둘째 자리까지 반올림한다. 결제가 실패하면 주문 성공을 반환하지 않는다.

AI는 조건식을 짧게 만들고, 결제 호출을 예외 처리로 감싸고, 금액 계산을 별도 메서드로 옮겼다. 재고 클래스에서는 동기화도 제거했다. 변경 요청(pull request, PR)의 설명에는 다음과 같이 적혀 있다.

주문 조건을 정리하고 결제 예외를 처리했다. 금액 계산을 통일했으며 재고 갱신도 단순화했다. 기존 주문 동작은 유지된다.

설명만 보면 적절한 정리처럼 보인다. 그러나 “예외를 처리했다”는 말은 실패가 호출자에게 정확히 전달된다는 뜻이 아니다. “계산을 통일했다”는 말도 반올림 규칙을 보존했다는 증거가 아니다. 리뷰어는 설명에 쓰인 동사보다 반환값과 상태 변화를 확인해야 한다.

원래 있던 테스트는 재고가 넉넉하고 쿠폰 문자열이 있으며 결제가 성공하는 주문 하나만 확인했다. 이 입력에서는 원본과 AI 수정본이 모두 10.00을 반환한다. 팀은 이 한 건의 성공을 근거로 병합할지, 요구에서 빠진 입력을 더 실행할지 결정해야 한다.

요구를 먼저 놓고 diff를 읽는다

diff를 위에서 아래로만 읽으면 눈앞의 문법 수정에 집중하기 쉽다. 먼저 PR의 목적과 허용된 동작 변경을 확인한다. 이번 작업은 구조 정리이므로, 별도 합의가 없는 동작 차이는 검토 대상이다. 그다음 외부에 드러나는 결과부터 거꾸로 읽는다. 반환값, 예외, 공유 상태 갱신, 계산식, 조건식 순으로 따라가면 변경의 영향을 좁히기 쉽다.

파일 순서보다 호출 관계가 중요하다. 주문 메서드가 호출하는 금액 계산과 결제 모의 객체를 함께 보고, 재고 클래스가 여러 요청에서 공유되는지도 확인한다. 메서드 이름이 그대로여도 반환값의 의미나 동시 실행의 보장은 바뀔 수 있다.

요구와 AI 변경을 대조하면 확인할 입력이 드러난다
요구AI 변경확인할 입력기대 결과
마지막 재고도 주문 가능재고 부족 조건에 등호 추가재고 1, 수량 1주문 성공
null 쿠폰은 할인 없음쿠폰에서 직접 equals 호출쿠폰 null정상 금액
결제 실패는 주문 실패예외를 잡고 계속 진행결제 모의 객체가 예외 발생결제 오류 반환
최종 금액을 반올림버림 방식으로 변경10.01에 HALF 적용5.01
재고 1개는 한 명만 확보검사와 차감의 동기화 제거두 구매자가 재고 1을 읽음한 명만 성공

이 표는 변경 줄마다 비슷한 테스트를 붙이는 목록이 아니다. 팀이 지켜야 하는 계약을 코드 변경과 연결한 지도다. 변경되지 않은 줄도 영향을 받을 수 있다. 예를 들어 예외 뒤의 성공 반환문은 원래부터 있었더라도, 새로 추가한 catch 때문에 실패 경로에서도 실행될 수 있다.

요구에서 관찰할 결과를 정한 뒤 변경된 경로와 입력을 연결한다

리뷰 메모도 관찰 가능한 표현으로 쓴다. “조건이 이상하다”보다 “재고 1, 주문 수량 1에서 원본은 성공하지만 수정본은 재고 부족을 반환한다”가 낫다. 재현 입력, 실제 결과, 요구상 기대 결과가 있으면 작성자는 수정 방향을 판단할 수 있다. AI에게 재수정을 맡길 때도 같은 근거를 전달한다.

그럴듯한 코드에서 의심할 다섯 지점

경계와 null은 정상 경로 바로 옆에 있다

재고가 수량보다 작을 때 거절하는 조건과, 작거나 같을 때 거절하는 조건은 기호 하나만 다르다. 하지만 뒤의 조건은 마지막 재고 판매를 막는다. 경계에서는 기준값보다 작은 값, 같은 값, 큰 값을 나누어 본다. 이 장의 비교 입력은 특히 등호가 결과를 바꾸는 지점을 선택한다.

null도 입력 계약으로 판단한다. 이 서비스에서는 쿠폰 누락을 허용하므로 null을 할인 없음으로 해석한다. 모든 null을 빈 문자열로 바꾸는 식의 일괄 처리는 필요하지 않다. 값이 있어야 하는 필드까지 누락을 감추면 다른 오류가 된다. 여기서는 상수 문자열을 기준으로 비교해 쿠폰 누락만 안전하게 처리한다.

예외 처리와 금액 계산은 결과의 의미를 바꾼다

결제 호출이 예외를 던졌는데 catch가 비어 있으면, 실행은 다음 문장으로 넘어간다. 뒤에 성공 반환이 있으면 결제 실패가 주문 성공으로 바뀐다. 로그를 추가하더라도 반환 의미가 그대로라면 문제는 남는다. 예제의 결제 모의 객체는 실패 시 IllegalStateException을 던지며, 올바른 구현은 이를 결제 오류 결과로 바꾼다.

금액은 BigDecimal을 쓴다고 자동으로 맞아지지 않는다. 문자열로 생성했는지, 어느 단계에서 자릿수를 줄이는지, 어떤 반올림 방식을 쓰는지까지 요구와 맞아야 한다. 예제는 단가와 수량을 곱하고 쿠폰을 적용한 뒤, 최종 금액에 HALF_UP을 적용한다. 10.01의 절반인 5.005가 5.01이 되어야 하므로 DOWN을 쓰면 차이가 드러난다. 이 소수 금액은 반올림 규칙을 드러내기 위한 예제 계약이다.

동시성은 실행 순서로 설명한다

재고를 읽고, 충분한지 검사하고, 차감하는 과정은 여러 단계다. 각 줄이 짧아도 전체가 하나의 작업으로 보호되지 않으면 두 구매자가 같은 재고를 보고 성공할 수 있다. 재고 1에서 A와 B가 모두 1을 읽고 각자 0을 쓰면, 저장된 재고는 0인데 성공한 주문은 두 건이다.

완성 코드는 실제 스레드의 실행 시점에 기대지 않고 이 순서를 직접 재생한다. 따라서 실행할 때마다 같은 결과가 나온다. 이 재생은 가능한 잘못된 순서를 보여 주는 증거이며, 모든 동시 실행을 검증하는 부하 시험은 아니다. 수정한 재고 메서드는 synchronized로 검사와 차감을 함께 보호한다. 그 보장은 같은 재고 객체를 공유하고 해당 메서드를 통해 접근할 때 적용된다.

두 구매자가 차감 전에 같은 재고를 읽으면 재고 한 개로 두 번 성공할 수 있다

완성 코드

한 파일에 비교 대상과 실행기를 담는다. 원본은 이번 구조 정리 전에 합의한 동작을 나타낸다. AI 수정본에는 리뷰할 오류를 의도적으로 넣었다. 수정 완료본은 원본을 호출하지 않고 별도로 구현했다. 세 구현을 같은 Case 객체로 실행하되 재고 객체는 매번 새로 만들어 실행 사이의 상태 오염을 막는다.

기대 결과도 입력 표에 직접 적는다. 원본과 같다는 사실만으로 정답을 판단하지 않기 위해서다. 원본에 오류가 있었다면 요구에 적힌 기대 결과를 기준으로 다시 논의해야 한다. 코드의 주석 번호는 뒤의 해설에서 읽을 구간을 가리킨다.

Main.java

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.List;

public class Main {
    // [1] 같은 입력과 요구상 기대 결과
    record Case(String name, int stock, int quantity,
                String price, String coupon, boolean paymentFails,
                boolean race, String expected) {
    }

    public static void main(String[] args) {
        List<Case> cases = List.of(
            new Case("normal", 3, 1, "10.00", "NONE",
                false, false, "OK 10.00"),
            new Case("last-stock", 1, 1, "10.00", "NONE",
                false, false, "OK 10.00"),
            new Case("null-coupon", 3, 1, "10.00", null,
                false, false, "OK 10.00"),
            new Case("payment-failure", 3, 1, "10.00", "NONE",
                true, false, "ERROR payment"),
            new Case("rounding", 3, 1, "10.01", "HALF",
                false, false, "OK 5.01"),
            new Case("stock-race", 1, 1, "10.00", "NONE",
                false, true, "reserved=1, stock=0")
        );

        // [2] 차이를 출력하고 세 구현을 각각 검증한다.
        int differences = 0;
        for (Case c : cases) {
            String before = run(c, 0);
            String ai = run(c, 1);
            String fixed = run(c, 2);
            boolean different = !before.equals(ai);
            if (different) {
                differences++;
            }

            System.out.println(c.name() + " | original=" + before
                + " | ai=" + ai + " | fixed=" + fixed
                + " | " + (different ? "DIFF" : "SAME"));
            check(before.equals(c.expected()), c.name() + " original");
            check(ai.equals(c.expected()), c.name() + " ai");
            check(fixed.equals(c.expected()), c.name() + " fixed");
        }
        System.out.println("differences=" + differences);
        check(differences == 5, "known differences");
    }

    static void check(boolean condition, String name) {
        System.out.println((condition ? "PASS " : "FAIL ") + name);
    }

    // [3] 예상하지 못한 실행 예외도 비교 가능한 결과로 만든다.
    static String run(Case c, int version) {
        try {
            if (c.race()) {
                return race(version);
            }
            return switch (version) {
                case 0 -> original(c);
                case 1 -> aiModified(c);
                case 2 -> repaired(c);
                default -> throw new IllegalArgumentException("version");
            };
        } catch (RuntimeException e) {
            return "ERROR " + e.getClass().getSimpleName();
        }
    }

    // [4] 원본: 이번 작업에서 보존할 동작
    static String original(Case c) {
        if (c.quantity() <= 0) {
            return "REJECT quantity";
        }
        if (c.stock() < c.quantity()) {
            return "REJECT stock";
        }
        BigDecimal total = new BigDecimal(c.price())
            .multiply(BigDecimal.valueOf(c.quantity()));
        if ("HALF".equals(c.coupon())) {
            total = total.divide(BigDecimal.valueOf(2));
        }
        total = total.setScale(2, RoundingMode.HALF_UP);
        try {
            pay(c.paymentFails());
        } catch (IllegalStateException e) {
            return "ERROR payment";
        }
        return "OK " + total.toPlainString();
    }

    // [5] AI 수정본: 자연스러워 보여도 동작이 달라진다.
    static String aiModified(Case c) {
        if (c.quantity() <= 0) {
            return "REJECT quantity";
        }
        if (c.stock() <= c.quantity()) {
            return "REJECT stock";
        }
        BigDecimal total = new BigDecimal(c.price())
            .multiply(BigDecimal.valueOf(c.quantity()));
        if (c.coupon().equals("HALF")) {
            total = total.divide(BigDecimal.valueOf(2));
        }
        total = total.setScale(2, RoundingMode.DOWN);
        try {
            pay(c.paymentFails());
        } catch (IllegalStateException e) {
            // 실패를 반환하지 않아 아래의 성공 경로로 흘러간다.
        }
        return "OK " + total.toPlainString();
    }

    // [6] 수정 완료본: 원본과 별도로 구현한다.
    static String repaired(Case c) {
        int requested = c.quantity();
        if (requested < 1) {
            return "REJECT quantity";
        }
        if (requested > c.stock()) {
            return "REJECT stock";
        }

        BigDecimal amount = new BigDecimal(c.price())
            .multiply(BigDecimal.valueOf(requested));
        boolean halfCoupon = "HALF".equals(c.coupon());
        if (halfCoupon) {
            amount = amount.divide(new BigDecimal("2"));
        }
        BigDecimal charged = amount.setScale(2, RoundingMode.HALF_UP);

        try {
            pay(c.paymentFails());
            return "OK " + charged.toPlainString();
        } catch (IllegalStateException e) {
            return "ERROR payment";
        }
    }

    static void pay(boolean fails) {
        if (fails) {
            throw new IllegalStateException("mock payment failure");
        }
    }

    // [7] 원본 재고: 검사와 차감을 한 번에 보호한다.
    static final class OriginalStock {
        private int remaining = 1;

        synchronized boolean reserve() {
            if (remaining < 1) {
                return false;
            }
            remaining--;
            return true;
        }

        synchronized int remaining() {
            return remaining;
        }
    }

    // [8] 동기화가 사라진 구현의 읽기와 쓰기를 드러낸다.
    static final class AiStock {
        private int remaining = 1;

        int read() {
            return remaining;
        }

        boolean reserveFrom(int observed) {
            if (observed < 1) {
                return false;
            }
            remaining = observed - 1;
            return true;
        }
    }

    // [9] 수정 재고: 같은 객체의 요청을 직렬화한다.
    static final class RepairedStock {
        private int remaining = 1;

        synchronized boolean reserve() {
            if (remaining == 0) {
                return false;
            }
            remaining -= 1;
            return true;
        }

        synchronized int remaining() {
            return remaining;
        }
    }

    // [10] AI 구현에서 가능한 순서를 결정적으로 재생한다.
    static String race(int version) {
        if (version == 0) {
            OriginalStock stock = new OriginalStock();
            boolean a = stock.reserve();
            boolean b = stock.reserve();
            return reservations(a, b, stock.remaining());
        }
        if (version == 1) {
            AiStock stock = new AiStock();
            int observedA = stock.read();
            int observedB = stock.read();
            boolean a = stock.reserveFrom(observedA);
            boolean b = stock.reserveFrom(observedB);
            return reservations(a, b, stock.read());
        }
        if (version == 2) {
            RepairedStock stock = new RepairedStock();
            boolean a = stock.reserve();
            boolean b = stock.reserve();
            return reservations(a, b, stock.remaining());
        }
        throw new IllegalArgumentException("version");
    }

    static String reservations(boolean a, boolean b, int remaining) {
        int count = (a ? 1 : 0) + (b ? 1 : 0);
        return "reserved=" + count + ", stock=" + remaining;
    }
}

줄별 해설

[1] Case는 입력과 기대 결과를 묶는다. 쿠폰 null도 별도 분기 없이 데이터로 넣는다. race가 true인 행은 주문 금액 대신 공유 재고의 실행 순서를 검사한다. 서로 다른 종류의 검사를 한 실행기에 모았지만, 금액 검사와 재고 경쟁 검사가 하나의 주문 흐름을 구성하는 것은 아니다.

[2] 각 행을 원본, AI 수정본, 수정 완료본 순서로 실행한다. DIFF는 원본과 AI 수정본이 다르다는 뜻이다. PASS는 각 결과가 요구상 기대값과 같다는 뜻이다. 이 둘은 역할이 다르다. 두 구현이 함께 잘못되면 SAME이면서 FAIL일 수 있다. differences가 5인지 확인하는 마지막 검사는 이 예제에 넣은 차이가 모두 드러났는지 확인한다.

[3] run은 null 쿠폰에서 발생한 실행 예외를 문자열로 바꾼다. 그래야 한 입력의 예외 때문에 나머지 비교가 중단되지 않는다. 이는 비교 실행기의 편의 기능이다. 실제 주문 서비스가 예상하지 못한 예외를 모두 업무 결과 문자열로 바꾸라는 뜻은 아니다. 결제 실패는 각 구현 내부에서 따로 처리한다.

[4] 원본은 재고와 수량이 같을 때 허용하고, 상수 문자열에서 equals를 호출한다. 합계 계산 뒤에 반올림하며 결제 실패를 오류로 반환한다. 여기서 원본이 기준이 되는 이유는 오래된 코드이기 때문이 아니라, 이번 작업에서 유지하기로 한 요구와 일치하기 때문이다.

[5] AI 수정본의 오류는 짧은 줄에 흩어져 있다. 재고 비교의 등호, equals의 호출 대상, DOWN, 빈 catch를 각각 살펴본다. 정상 입력은 이 네 차이를 모두 지나가면서도 같은 결과를 낸다. 그래서 정상 사례만 실행하면 변경의 위험이 보이지 않는다.

[6] 수정 완료본은 비교 방향과 변수 구성을 조금 달리해 구현했다. 원본과 문자열이 같은지는 입력별 실행으로 확인한다. 다만 여섯 입력의 일치는 전체 입력 공간의 동등성을 증명하지 않는다. 가격 문자열의 형식 검증이나 주문 전체의 상태 관리는 이 예제에서 구현하지 않았다.

[7]~[9] 원본과 수정 재고는 검사와 차감을 같은 synchronized 메서드 안에 둔다. AiStock은 보호가 사라진 복합 연산을 읽기와 쓰기로 나누어 보여 준다. 이 객체의 read와 reserveFrom을 외부에 공개하는 설계 자체를 권장하는 것이 아니다. 두 요청이 어느 값을 읽고 쓰는지 관찰하기 위한 장치다.

[10] 원본과 수정 재고에서는 한 요청이 재고를 확보한 뒤 다음 요청이 0을 확인한다. AI 재고에서는 두 읽기를 먼저 수행한다. 이어서 두 요청이 각자 관찰한 1을 기준으로 성공한다. 시각, 난수, 스레드 스케줄러 없이 같은 실패를 재현하므로 출력이 고정된다.

실행 결과

Main.java를 빈 작업 디렉터리에 저장한 뒤 다음 명령으로 실행한다. JDK 17의 표준 라이브러리만 사용한다. 프로그램은 파일을 만들지 않으며, 외부 통신이나 추가 입력 없이 종료한다.

javac -d out *.java
java -cp out Main

예상 출력은 다음과 같다. AI 수정본의 FAIL 다섯 개는 의도적으로 넣은 결함을 잡아낸 결과다. 수정 완료본은 같은 입력에서 모두 PASS를 출력한다.

normal | original=OK 10.00 | ai=OK 10.00 | fixed=OK 10.00 | SAME
PASS normal original
PASS normal ai
PASS normal fixed
last-stock | original=OK 10.00 | ai=REJECT stock | fixed=OK 10.00 | DIFF
PASS last-stock original
FAIL last-stock ai
PASS last-stock fixed
null-coupon | original=OK 10.00 | ai=ERROR NullPointerException | fixed=OK 10.00 | DIFF
PASS null-coupon original
FAIL null-coupon ai
PASS null-coupon fixed
payment-failure | original=ERROR payment | ai=OK 10.00 | fixed=ERROR payment | DIFF
PASS payment-failure original
FAIL payment-failure ai
PASS payment-failure fixed
rounding | original=OK 5.01 | ai=OK 5.00 | fixed=OK 5.01 | DIFF
PASS rounding original
FAIL rounding ai
PASS rounding fixed
stock-race | original=reserved=1, stock=0 | ai=reserved=2, stock=0 | fixed=reserved=1, stock=0 | DIFF
PASS stock-race original
FAIL stock-race ai
PASS stock-race fixed
differences=5
PASS known differences

정상 주문의 SAME만 보고 병합했다면 나머지 다섯 차이는 남았을 것이다. 반대로 DIFF가 있다는 이유만으로 곧바로 오류라고 단정하지도 않는다. 기능 변경이 목적이었다면 차이가 요구에 맞을 수 있다. 이번에는 기존 동작 유지가 목적이고, 기대 결과도 합의한 계약에서 나왔으므로 수정 근거가 된다.

이 실행기는 비교 결과를 읽기 위한 예제라서 FAIL을 출력해도 예외를 던지지 않는다. 따라서 프로세스의 정상 종료를 테스트 성공과 같은 뜻으로 해석하면 안 된다. 다음 장에서는 사람이 읽는 결과와 기계가 차단할 조건을 구분해 다룬다.

실무에서 자주 틀리는 것

다음 코드는 완성 코드의 문제 구간을 각각 독립 실행할 수 있게 줄인 예다. 각 클래스를 클래스 이름과 같은 파일에 저장하면 main으로 전후 결과를 확인할 수 있다. Wrong은 문제 구현, Fixed는 수정 구현의 결과를 뜻한다.

등호와 null 처리를 단순한 스타일 변경으로 본다

두 변경 모두 짧아서 리뷰에서 지나치기 쉽다. 다음 코드는 같은 재고 입력과 같은 null 쿠폰으로 전후를 비교한다. 넉넉한 재고나 문자열 쿠폰으로 입력을 바꾸면 오류를 놓칠 수 있다.

public class BoundaryNullExample {
    static boolean stockWrong(int stock, int quantity) {
        return stock > quantity;
    }

    static boolean stockFixed(int stock, int quantity) {
        return stock >= quantity;
    }

    static boolean couponWrong(String coupon) {
        return coupon.equals("HALF");
    }

    static boolean couponFixed(String coupon) {
        return "HALF".equals(coupon);
    }

    public static void main(String[] args) {
        System.out.println("stockWrong=" + stockWrong(1, 1));
        System.out.println("stockFixed=" + stockFixed(1, 1));
        try {
            System.out.println("couponWrong=" + couponWrong(null));
        } catch (NullPointerException e) {
            System.out.println("couponWrong=NullPointerException");
        }
        System.out.println("couponFixed=" + couponFixed(null));
    }
}

실행 결과는 stockWrong=false, stockFixed=true, couponWrong=NullPointerException, couponFixed=false 순이다. 수정 후 null 쿠폰의 false는 실패가 아니라 HALF 할인을 적용하지 않는다는 뜻이다. 반환값의 의미까지 읽어야 한다.

catch가 있으면 실패 처리가 끝났다고 생각한다

빈 catch는 예외를 감추고 성공 반환을 실행하게 한다. 같은 실패 입력을 넣어도 두 메서드의 반환값이 다르다. 수정은 catch의 존재가 아니라 실패 경로의 결과로 확인한다.

public class PaymentExample {
    static void pay(boolean fails) {
        if (fails) {
            throw new IllegalStateException("payment");
        }
    }

    static String wrong(boolean fails) {
        try {
            pay(fails);
        } catch (IllegalStateException e) {
        }
        return "OK";
    }

    static String fixed(boolean fails) {
        try {
            pay(fails);
            return "OK";
        } catch (IllegalStateException e) {
            return "ERROR payment";
        }
    }

    public static void main(String[] args) {
        System.out.println("wrong=" + wrong(true));
        System.out.println("fixed=" + fixed(true));
    }
}

출력은 wrong=OK와 fixed=ERROR payment다. 실제 서비스에서는 실패 이후 주문 상태도 확인해야 한다. 이 예제는 결제 실패를 성공으로 보고하지 않는 반환 계약에 범위를 한정한다.

BigDecimal을 쓰면 반올림도 맞다고 생각한다

자료형과 업무 규칙은 별개다. 다음 두 메서드는 같은 값을 받아도 자릿수를 줄이는 방식 때문에 다른 금액을 반환한다. 반올림 규칙의 이름을 코드와 요구 대조표 양쪽에 적어 두면 변경을 확인하기 쉽다.

import java.math.BigDecimal;
import java.math.RoundingMode;

public class RoundingExample {
    static BigDecimal wrong(BigDecimal amount) {
        return amount.setScale(2, RoundingMode.DOWN);
    }

    static BigDecimal fixed(BigDecimal amount) {
        return amount.setScale(2, RoundingMode.HALF_UP);
    }

    public static void main(String[] args) {
        BigDecimal amount = new BigDecimal("5.005");
        System.out.println("wrong=" + wrong(amount).toPlainString());
        System.out.println("fixed=" + fixed(amount).toPlainString());
    }
}

출력은 wrong=5.00과 fixed=5.01이다. 이 사례에서는 HALF_UP이 요구다. 다른 서비스에 같은 방식을 그대로 적용하기보다, 합계 전후 중 언제 반올림하는지까지 계약으로 확인한다.

최종 재고만 보고 동시성 문제를 놓친다

재고가 음수가 아니어도 초과 판매가 가능하다. 완성 코드의 AI 재고는 최종 값이 0이므로 재고 숫자만 검사하면 통과한다. 성공한 예약 수까지 함께 관찰해야 한다.

public class StockExample {
    private int stock = 1;

    boolean wrong(int observed) {
        if (observed < 1) {
            return false;
        }
        stock = observed - 1;
        return true;
    }

    synchronized boolean fixed() {
        if (stock < 1) {
            return false;
        }
        stock--;
        return true;
    }

    public static void main(String[] args) {
        StockExample before = new StockExample();
        int a = before.stock;
        int b = before.stock;
        boolean first = before.wrong(a);
        boolean second = before.wrong(b);
        System.out.println("wrong=" + first + "," + second
            + ",stock=" + before.stock);

        StockExample after = new StockExample();
        first = after.fixed();
        second = after.fixed();
        System.out.println("fixed=" + first + "," + second
            + ",stock=" + after.stock);
    }
}

출력은 wrong=true,true,stock=0과 fixed=true,false,stock=0이다. 최종 재고는 같고 성공 횟수만 다르다. 이 수정은 한 프로세스의 같은 객체에 대한 보호다. 저장소가 여러 실행 인스턴스에 걸쳐 공유된다면 그 공유 지점에 맞는 별도 설계가 필요하다.

한눈에 보기

리뷰 판단은 코드의 모양보다 같은 입력에서 관찰한 결과에 근거한다
관찰 지점놓치기 쉬운 변경확인 방법
경계 조건비교식의 등호기준값과 같은 입력 실행
null 계약메서드 호출 대상 변경누락을 허용한 필드에 null 입력
실패 경로예외 뒤 성공으로 진행모의 객체에서 실패 발생
금액반올림 방식과 적용 시점버릴 자리의 값이 결과를 바꾸는 금액 사용
공유 상태검사와 갱신의 보호 해제같은 값을 두 번 읽는 순서 재생
수정 확인정상 사례만 재실행원본·AI·수정본에 같은 입력 목록 적용

리뷰를 마칠 때는 “읽어 보니 괜찮다” 대신 어떤 요구를 어떤 입력으로 확인했는지 남긴다. 이 예제에서는 정상 입력 한 건은 같았고, 다섯 입력은 AI 수정본에서 달랐으며, 수정 완료본은 모든 기대값과 같았다. 이것이 이번 실행으로 말할 수 있는 범위다.

연습 문제

  1. 완성 코드의 입력 목록에 재고 0, 수량 1인 행과 재고 3, 수량 0인 행을 추가한다. 기대 결과를 먼저 정하고 세 구현의 결과를 비교한다. 두 행에서 차이가 없으면 기존 오류가 해결되었다고 말할 수 있는지도 설명한다.
  2. rounding 행의 단가를 10.02로 바꾼 복사본을 추가한다. HALF 적용 후 세 구현의 금액과 differences의 변화를 예상한다. 이 입력만으로 반올림 오류를 찾을 수 있는지 설명한다.
  3. stock-race에서 최종 재고만 기대값으로 검사하도록 바꾸었다고 가정한다. 어떤 오류를 놓치는가. 예약 성공 수와 재고 사이에 지켜야 할 관계를 문장으로 적는다.
  4. 원본과 AI 수정본이 모두 결제 실패를 OK로 반환한다고 가정한다. SAME 표시와 기대 결과 검사는 각각 무엇을 알려 주는가. 이때 리뷰 메모에 남길 근거를 작성한다.

정답과 해설

  1. 재고 0, 수량 1의 기대 결과는 REJECT stock이고, 수량 0의 기대 결과는 REJECT quantity다. 세 구현 모두 이 결과를 낸다. 추가한 입력에서 같다는 사실은 해당 두 계약을 확인한 것이며, 마지막 재고나 null 등 다른 입력의 오류를 해결했다는 뜻은 아니다. 기존 다섯 차이는 그대로 남으므로 differences는 5다.
  2. 10.02의 절반은 5.01이다. 원본, AI 수정본, 수정 완료본 모두 OK 5.01을 반환한다. 기존 행을 남기고 복사본만 추가했다면 differences는 여전히 5다. 이 금액에서는 버림과 반올림 결과가 같으므로 반올림 방식의 차이를 검출하지 못한다. 입력은 기능 이름만 대표할 것이 아니라 오류가 드러나는 값이어야 한다.
  3. 두 구매자가 모두 성공했는데 재고가 0인 오류를 놓친다. 이 예제처럼 예약 한 건이 재고 한 개를 사용하고 취소나 보충이 없다면, 초기 재고는 성공한 예약 수와 남은 재고의 합과 같아야 한다. AI 재고에서는 초기 재고가 1인데 성공 수 2와 남은 재고 0의 합은 2이므로 관계가 깨진다.
  4. SAME은 두 구현의 결과가 같다는 사실만 알려 준다. 요구상 기대값이 ERROR payment라면 두 구현의 기대 결과 검사는 모두 FAIL이다. 리뷰 메모에는 “결제 모의 객체가 예외를 던지는 입력에서 두 구현 모두 OK를 반환한다. 결제 실패는 주문 성공으로 반환하지 않는다는 요구와 다르다”라고 남긴다. 원본의 동작도 수정 대상인지 팀이 명확히 정하고, 합의한 기대값으로 수정본을 다시 실행한다.

댓글 0

아직 댓글이 없습니다. 첫 댓글을 남겨 보세요.

댓글을 남기려면 로그인이 필요합니다.