[Design Patterns] 23. 패턴을 활용한 코드 리뷰와 설계 리뷰

  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131

import java.util.ArrayList;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.transaction.annotation.Transactional;

/**
 * 코드 리뷰 체크리스트 - 패턴 적용 검증
 * 
 * □ 패턴 적용 적절성
 *   - 문제 상황과 패턴이 매치하는가?
 *   - 패턴의 Intent와 실제 사용 목적이 일치하는가?
 *   - 더 간단한 해결책은 없는가?
 * 
 * □ 패턴 구현 완전성
 *   - 패턴의 필수 구성 요소가 모두 구현되었는가?
 *   - 패턴의 협력 관계가 올바르게 표현되었는가?
 *   - 패턴의 변형이 적절하게 적용되었는가?
 * 
 * □ 코드 품질
 *   - 네이밍이 패턴의 의도를 명확히 드러내는가?
 *   - 추상화 수준이 일관되는가?
 *   - 단일 책임 원칙이 지켜졌는가?
 */

// 리뷰 대상 코드가 의존하는 최소 도메인 타입 (실제 프로젝트에서는 별도 파일로 분리)
enum OrderStatus {
    PENDING, CONFIRMED, CANCELLED
}

class Order {
    private final Long id;
    private OrderStatus status;

    public Order(Long id, OrderStatus status) {
        this.id = id;
        this.status = status;
    }

    public void setStatus(OrderStatus status) {
        this.status = status;
    }

    public OrderStatus getStatus() {
        return status;
    }

    // 불변 스타일 API: 상태를 바꾼 새 인스턴스를 반환
    public Order confirm() {
        return new Order(this.id, OrderStatus.CONFIRMED);
    }
}

interface OrderObserver {
    void onOrderUpdated(Order order);
}

interface OrderRepository {
    Order save(Order order);
}

// 리뷰 예시 1: Observer 패턴 적용 검토
public class OrderService {
    private List<OrderObserver> observers = new ArrayList<>(); // 올바른 패턴 구현

    @Autowired
    private OrderRepository orderRepository;
    
    public void addObserver(OrderObserver observer) {
        observers.add(observer);
    }
    
    public void removeObserver(OrderObserver observer) {
        observers.remove(observer);
    }
    
    // 리뷰 포인트: notifyObservers가 private으로 숨겨져 있어 패턴 의도 불분명
    private void notifyObservers(Order order) {
        for (OrderObserver observer : observers) {
            observer.onOrderUpdated(order);
        }
    }
    
    public void confirmOrder(Order order) {
        order.setStatus(OrderStatus.CONFIRMED);
        orderRepository.save(order);
        
        // 올바른 알림 호출
        notifyObservers(order);
    }
}

// 리뷰 개선 제안
public class ImprovedOrderService {
    private static final Logger log = LoggerFactory.getLogger(ImprovedOrderService.class);

    private final List<OrderObserver> observers = new ArrayList<>();

    @Autowired
    private OrderRepository orderRepository;
    
    public void addObserver(OrderObserver observer) {
        if (observer != null && !observers.contains(observer)) { // 중복 방지
            observers.add(observer);
        }
    }
    
    public void removeObserver(OrderObserver observer) {
        observers.remove(observer);
    }
    
    // protected로 변경하여 확장 가능성 열어둠
    protected void notifyObservers(Order order) {
        // 예외 안전성 추가
        for (OrderObserver observer : new ArrayList<>(observers)) {
            try {
                observer.onOrderUpdated(order);
            } catch (Exception e) {
                log.error("Observer notification failed", e);
                // 관찰자 오류가 전체 프로세스를 중단하지 않도록
            }
        }
    }
    
    @Transactional
    public void confirmOrder(Order order) {
        Order savedOrder = orderRepository.save(order.confirm());
        notifyObservers(savedOrder); // 저장 후 알림
    }
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78

import java.util.List;

// 리뷰 기법 1: 패턴 인식 및 명명
public class PatternRecognitionReview {

    // 아래 9개 메서드는 실제 파일 I/O를 생략한 스텁입니다. 이 예시의 핵심은
    // "조건 분기 뒤에 숨은 Strategy 패턴을 찾아내는 리뷰 시각"이지 파싱 로직 자체가 아니므로,
    // 컴파일 가능성만 보장하는 최소 동작(로그 출력)만 남겨 둡니다.
    private static void readTextFile(String filePath) { System.out.println("read text: " + filePath); }
    private static void parseTextContent() { System.out.println("parse text content"); }
    private static void generateTextReport() { System.out.println("generate text report"); }
    private static void readCsvFile(String filePath) { System.out.println("read csv: " + filePath); }
    private static void parseCsvContent() { System.out.println("parse csv content"); }
    private static void generateCsvReport() { System.out.println("generate csv report"); }
    private static void readJsonFile(String filePath) { System.out.println("read json: " + filePath); }
    private static void parseJsonContent() { System.out.println("parse json content"); }
    private static void generateJsonReport() { System.out.println("generate json report"); }

    // Before: 패턴이 숨겨진 코드
    public static class LegacyFileProcessor {
        public void processFile(String filePath) {
            if (filePath.endsWith(".txt")) {
                // 텍스트 파일 처리
                readTextFile(filePath);
                parseTextContent();
                generateTextReport();
            } else if (filePath.endsWith(".csv")) {
                // CSV 파일 처리
                readCsvFile(filePath);
                parseCsvContent();
                generateCsvReport();
            } else if (filePath.endsWith(".json")) {
                // JSON 파일 처리
                readJsonFile(filePath);
                parseJsonContent();
                generateJsonReport();
            }
        }
    }
    
    // 리뷰 포인트: "이 코드에서 Strategy 패턴을 발견할 수 있습니다"
    // After: 패턴을 명시적으로 적용 (LegacyFileProcessor를 대체)
    public interface FileProcessingStrategy {
        boolean supports(String filePath);
        void process(String filePath);
    }
    
    public static class TextFileProcessingStrategy implements FileProcessingStrategy {
        @Override
        public boolean supports(String filePath) {
            return filePath.endsWith(".txt");
        }
        
        @Override  
        public void process(String filePath) {
            readTextFile(filePath);
            parseTextContent();
            generateTextReport();
        }
    }
    
    public static class FileProcessor {
        private final List<FileProcessingStrategy> strategies;
        
        public FileProcessor(List<FileProcessingStrategy> strategies) {
            this.strategies = strategies;
        }
        
        public void processFile(String filePath) {
            FileProcessingStrategy strategy = strategies.stream()
                .filter(s -> s.supports(filePath))
                .findFirst()
                .orElseThrow(() -> new UnsupportedOperationException("Unsupported file type: " + filePath));
                
            strategy.process(filePath);
        }
    }
}

  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

// 정적 분석 규칙에 필요한 최소 타입 정의 (실제 프로젝트에서는 정적 분석기 API가 제공)
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@interface AnalysisRule {
}

enum IssueSeverity { ERROR, WARNING }

// Issue·ClassInfo처럼 "필드 조합 자체가 값의 전부"인 타입은 생성자·getter를 직접 적을 필요가 없습니다.
// Java 16부터 정식 도입된 record는 필드 선언만으로 생성자·접근자·equals/hashCode/toString을 컴파일러가 만들어 주므로,
// 아래 여섯 개 DTO는 모두 record로 선언해 이 검증 로직의 핵심(SingletonPatternRule.validate)에 시선이 머물게 합니다.
record Issue(IssueSeverity severity, String message) {
    static Issue error(String message) { return new Issue(IssueSeverity.ERROR, message); }
    static Issue warning(String message) { return new Issue(IssueSeverity.WARNING, message); }
}

// 리플렉션/AST 분석 결과를 감싸는 클래스 메타데이터. boolean 필드의 record 접근자는
// isXxx()가 아니라 필드명 그대로(privateConstructor(), threadSafe())가 생성된다는 점에 주의합니다.
record ClassInfo(String className, List<String> staticFields, List<String> methodNames,
                  boolean privateConstructor, boolean threadSafe) {
    boolean hasStaticField(String name) { return staticFields.contains(name); }
    boolean hasMethod(String name) { return methodNames.contains(name); }
}

// 커스텀 정적 분석 규칙
public class PatternValidationRules {
    
    /**
     * 규칙 1: Singleton 패턴 검증
     * - private 생성자 확인
     * - getInstance() 메서드 존재 확인
     * - 스레드 안전성 검증
     */
    @AnalysisRule
    public class SingletonPatternRule {
        
        public List<Issue> validate(ClassInfo classInfo) {
            List<Issue> issues = new ArrayList<>();
            
            if (isSingletonPattern(classInfo)) {
                // private 생성자 확인
                if (!hasPrivateConstructor(classInfo)) {
                    issues.add(Issue.error("Singleton must have private constructor"));
                }
                
                // getInstance 메서드 확인
                if (!hasGetInstanceMethod(classInfo)) {
                    issues.add(Issue.error("Singleton must have getInstance() method"));
                }
                
                // 스레드 안전성 확인
                if (!isThreadSafe(classInfo)) {
                    issues.add(Issue.warning("Singleton should be thread-safe"));
                }
            }
            
            return issues;
        }
        
        private boolean isSingletonPattern(ClassInfo classInfo) {
            return classInfo.hasStaticField("instance") && 
                   classInfo.hasMethod("getInstance");
        }

        private boolean hasPrivateConstructor(ClassInfo classInfo) {
            return classInfo.privateConstructor();
        }

        private boolean hasGetInstanceMethod(ClassInfo classInfo) {
            return classInfo.hasMethod("getInstance");
        }

        private boolean isThreadSafe(ClassInfo classInfo) {
            return classInfo.threadSafe();
        }
    }
}

// 자동화 워크플로우가 주고받는 입출력 타입. 아래 네 개도 필드 조합이 곧 값이므로 record로 선언합니다.
// 다만 ReviewReport는 addSection() 호출마다 내부 Map 상태가 바뀌는 가변 객체이므로 record로 표현할 수 없어 class로 남깁니다.
record CodeBase(List<ClassInfo> classes) {}
record DetectedPattern(String patternName, String className) {}
record PatternValidationResult(DetectedPattern pattern, List<Issue> issues) {}
record CodeQualityMetrics(int cyclomaticComplexity, double testCoverage) {}
record ImprovementSuggestion(String description, String targetClassName) {}

class ReviewReport {
    private final Map<String, Object> sections = new LinkedHashMap<>();

    public void addSection(String name, Object content) {
        sections.put(name, content);
    }
}

interface PatternDetector {
    List<DetectedPattern> detectPatterns(CodeBase codeBase);
}

interface PatternValidator {
    List<PatternValidationResult> validatePatterns(List<DetectedPattern> patterns);
}

interface MetricsCalculator {
    CodeQualityMetrics calculate(CodeBase codeBase);
}

interface SuggestionEngine {
    List<ImprovementSuggestion> generateSuggestions(
        List<DetectedPattern> patterns,
        List<PatternValidationResult> validations,
        CodeQualityMetrics metrics);
}

// 리뷰 자동화 워크플로우
@Service
public class AutomatedReviewService {

    @Autowired
    private PatternDetector patternDetector;

    @Autowired
    private PatternValidator patternValidator;

    @Autowired
    private MetricsCalculator metricsCalculator;

    @Autowired
    private SuggestionEngine suggestionEngine;
    
    public ReviewReport conductAutomatedReview(CodeBase codeBase) {
        ReviewReport report = new ReviewReport();
        
        // 1. 패턴 적용 탐지
        List<DetectedPattern> patterns = patternDetector.detectPatterns(codeBase);
        report.addSection("Detected Patterns", patterns);
        
        // 2. 패턴 구현 검증
        List<PatternValidationResult> validations = patternValidator.validatePatterns(patterns);
        report.addSection("Pattern Validations", validations);
        
        // 3. 코드 품질 메트릭
        CodeQualityMetrics metrics = metricsCalculator.calculate(codeBase);
        report.addSection("Quality Metrics", metrics);
        
        // 4. 개선 제안 생성
        List<ImprovementSuggestion> suggestions = suggestionEngine.generateSuggestions(
            patterns, validations, metrics);
        report.addSection("Improvement Suggestions", suggestions);
        
        return report;
    }
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.springframework.stereotype.Component;

@Component
public class ReviewQualityAnalyzer {
    
    // 리뷰 효과성 측정
    public ReviewEffectivenessReport analyzeReviewEffectiveness(
            List<CodeReview> reviews, TimeRange timeRange) {
        
        ReviewEffectivenessReport report = new ReviewEffectivenessReport();
        
        // 1. 결함 발견율
        double defectDetectionRate = calculateDefectDetectionRate(reviews, timeRange);
        report.setDefectDetectionRate(defectDetectionRate);
        
        // 2. 패턴 적용 개선율
        double patternImprovementRate = calculatePatternImprovementRate(reviews);
        report.setPatternImprovementRate(patternImprovementRate);
        
        // 3. 코드 품질 향상도
        CodeQualityTrend qualityTrend = analyzeCodeQualityTrend(timeRange);
        report.setQualityTrend(qualityTrend);
        
        // 4. 리뷰 참여도
        ReviewParticipationMetrics participation = calculateParticipationMetrics(reviews);
        report.setParticipationMetrics(participation);
        
        return report;
    }
    
    // 패턴별 리뷰 성과 분석
    public Map<PatternType, PatternReviewMetrics> analyzePatternReviewMetrics(
            List<CodeReview> reviews) {
        
        Map<PatternType, PatternReviewMetrics> metrics = new HashMap<>();
        
        for (PatternType patternType : PatternType.values()) {
            List<CodeReview> patternReviews = reviews.stream()
                .filter(review -> review.involvePattern(patternType))
                .collect(Collectors.toList());
                
            if (!patternReviews.isEmpty()) {
                PatternReviewMetrics patternMetrics = PatternReviewMetrics.builder()
                    .totalReviews(patternReviews.size())
                    .averageReviewTime(calculateAverageReviewTime(patternReviews))
                    .defectsFound(countDefectsFound(patternReviews))
                    .improvementsSuggested(countImprovementsSuggested(patternReviews))
                    .implementationQualityScore(calculateImplementationQuality(patternReviews))
                    .build();
                    
                metrics.put(patternType, patternMetrics);
            }
        }
        
        return metrics;
    }
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

// 엔터프라이즈 리뷰 프레임워크
@Configuration
public class EnterpriseReviewFramework {
    
    // 팀별 리뷰 정책 설정
    @Bean
    public ReviewPolicyManager reviewPolicyManager() {
        ReviewPolicyManager manager = new ReviewPolicyManager();
        
        // 백엔드 팀 정책
        ReviewPolicy backendPolicy = ReviewPolicy.builder()
            .teamName("Backend")
            .requiredReviewers(2)
            .mandatoryPatternCheck(true)
            .patternComplexityThreshold(PatternComplexity.MEDIUM)
            .codeQualityGate(CodeQualityGate.STRICT)
            .build();
            
        // 프론트엔드 팀 정책  
        ReviewPolicy frontendPolicy = ReviewPolicy.builder()
            .teamName("Frontend")
            .requiredReviewers(1)
            .mandatoryPatternCheck(false)
            .patternComplexityThreshold(PatternComplexity.LOW)
            .codeQualityGate(CodeQualityGate.STANDARD)
            .build();
            
        manager.addPolicy("backend", backendPolicy);
        manager.addPolicy("frontend", frontendPolicy);
        
        return manager;
    }
    
    // 자동화된 리뷰 워크플로우
    @Bean
    public ReviewWorkflowEngine reviewWorkflowEngine() {
        return ReviewWorkflowEngine.builder()
            .addStage(new AutomatedPatternAnalysisStage())
            .addStage(new CodeQualityCheckStage())
            .addStage(new SecurityScanStage())
            .addStage(new HumanReviewStage())
            .addStage(new ApprovalStage())
            .build();
    }
}