237 refactor profile background color refactor#229
237 refactor profile background color refactor#229parkjuyeong0312 wants to merge 2 commits intomainfrom
Conversation
Walkthrough프로필 배경색 초기 데이터 구성에서 문자열 타입을 enum 기반으로 변경하고, ColorType enum을 신설했다. 서비스 레이어는 확률 기반 타입 결정 → 타입별 색상 조회 → 무작위 선택의 단계적 흐름으로 리팩터링되었고, 이름 검색 메서드 파라미터 명이 정돈되었다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as ProfileBackgroundColorService
participant Repo as ProfileBackgroundColorRepository
Client->>Service: pickColor()
Service->>Service: determineColorTypeByProbability()
Service->>Repo: findByType(colorType.getValue())
Repo-->>Service: List<ProfileBackgroundColor>
Service->>Service: selectRandomColor(list)
Service-->>Client: ProfileBackgroundColor
sequenceDiagram
participant Client
participant Service as ProfileBackgroundColorService
participant Repo as ProfileBackgroundColorRepository
Client->>Service: getColorByName(colorName)
Service->>Repo: findByName(colorName)
Repo-->>Service: Optional<ProfileBackgroundColor>
Service-->>Client: ProfileBackgroundColor or Exception
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
src/main/java/com/aloc/aloc/profilebackgroundcolor/enums/ColorType.java (2)
3-6: 주석(확률)과 구현(서비스의 임계값) 간 드리프트 방지 제안주석에 85/10/5 확률이 명시되어 있고, 서비스 클래스에는 COMMON_THRESHOLD/RARE_THRESHOLD가 별도로 존재합니다. 시간이 지나면 문서-코드가 어긋날 가능성이 있어, 가중치를 Enum 내부로 끌어올리는 방식을 고려해보세요.
예시(diff는 참조용, 실제 반영은 선택):
public enum ColorType { - COMMON("common"), - RARE("rare"), - SPECIAL("special"); + COMMON("common", 85), + RARE("rare", 10), + SPECIAL("special", 5); - private final String value; + private final String value; + private final int weightPercent; - ColorType(String value) { - this.value = value; - } + ColorType(String value, int weightPercent) { + this.value = value; + this.weightPercent = weightPercent; + } public String getValue() { return value; } + + public int getWeightPercent() { + return weightPercent; + } }이렇게 두면, 서비스에서는 누적 가중치 기반으로 타입을 추출할 수 있어 주석/코드 드리프트를 예방할 수 있습니다.
12-20: toString 오버라이드 또는 fromValue 제공 고려현재는 getValue()로 문자열을 노출합니다. 호출부에서 value 문자열이 필요할 때마다 getValue() 호출이 반복됩니다. toString() 오버라이드(혹은 fromValue(String) 역매핑) 추가를 고려하면 가독성과 사용성 모두 좋아집니다.
예시:
public String getValue() { return value; } + + @Override + public String toString() { + return value; + } + + public static ColorType fromValue(String value) { + for (ColorType ct : values()) { + if (ct.value.equals(value)) return ct; + } + throw new IllegalArgumentException("Unknown ColorType value: " + value); + }src/main/java/com/aloc/aloc/global/init/DevDataInitializer.java (4)
108-116: 빈 문자열("") 대신 null 사용 권장 (데이터 일관성)SPECIAL 색상에서 color4, color5에 빈 문자열이 들어갑니다. 엔티티 필드는 null 허용이고, 다른 항목은 null을 사용하고 있어 일관성이 떨어집니다. API 응답에서도 ""와 null 구분이 생겨 클라이언트 처리 복잡도가 증가합니다. null로 통일을 권장합니다.
제안 diff:
new ProfileBackgroundColor( "BeautifulYPB", "#FFB800", "#FF69F0", "#408CFF", - "", - "", + null, + null, ColorType.SPECIAL.getValue(), 135),
117-124: 위와 동일: 빈 문자열("") → null 통일 권장데이터 일관성을 위해 color4, color5를 null로 정리해주세요.
제안 diff:
new ProfileBackgroundColor( "BeautifulBPR", "#408CFF", "#E95FFF", "#FF5A5A", - "", - "", + null, + null, ColorType.SPECIAL.getValue(), 135),
31-141: 가독성: 다인자 생성자 대신 Builder 사용 고려8개 파라미터 순서 의존이 강하고 실수 여지가 큽니다. 엔티티에 @builder가 이미 적용되어 있으므로 초기 데이터 생성도 Builder로 전환하면 가독성과 안전성이 올라갑니다.
예시:
ProfileBackgroundColor.builder() .name("Red") .color1("#FF5A5A") .type(ColorType.COMMON.getValue()) .build();
25-30: 초기 데이터 보강 전략 재고 (선택 사항)count()>0이면 전체 생성을 건너뜁니다. 기존 dev DB에 일부만 존재하는 경우 신규 색상이 추가되지 않을 수 있습니다. 이름(@id) 기준으로 존재하지 않는 항목만 upsert하는 로직으로 바꾸면 개발 편의성이 좋아집니다.
src/main/java/com/aloc/aloc/profilebackgroundcolor/service/ProfileBackgroundColorService.java (4)
64-72: 빈 리스트 예외 처리 OK, 메시지 구체화는 선택 사항타입별 컬러가 비어있을 때 IllegalStateException으로 빠르게 실패하는 전략 좋습니다. 추가로, 운영 관점에서 원인 파악을 돕도록 타입과 현재 DB 총량(혹은 타입별 건수)을 포함해도 좋습니다.
예시:
- throw new IllegalStateException("선택된 타입에 해당하는 색상이 없습니다: " + colorType.getValue()); + throw new IllegalStateException( + "선택된 타입에 해당하는 색상이 없습니다: " + colorType.getValue() + + " (colors.size=" + colors.size() + ")" + );Also applies to: 77-79
20-20: Random 사용 방식 개선 제안: ThreadLocalRandom 또는 주입형 Random현재 필드에 new Random()을 고정 생성합니다. 멀티스레드/성능을 고려하면 ThreadLocalRandom 사용이 더 경량이며, 테스트 용이성을 고려하면 Random을 주입받아 시드 고정이 가능합니다. 상황에 맞게 두 가지 중 하나를 권장합니다.
옵션 A: ThreadLocalRandom 사용(간단, 고성능)
-import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; ... - private final Random randomNumberGenerator = new Random(); + // ThreadLocalRandom 사용으로 락 경합 없고 성능 우수 ... - int randomNumber = randomNumberGenerator.nextInt(MAX_PROBABILITY) + 1; + int randomNumber = ThreadLocalRandom.current().nextInt(MAX_PROBABILITY) + 1; ... - return colors.get(randomNumberGenerator.nextInt(colors.size())).getName(); + return colors.get(ThreadLocalRandom.current().nextInt(colors.size())).getName();옵션 B: Random 주입(테스트에서 시드 고정 가능)
- 필드에서 초기화 제거 후 생성자 주입으로 교체(테스트 시
new Random(0)주입).- 별도 @configuration에서 Random 빈 정의.
Also applies to: 48-58, 77-79
22-26: 확률 상수 네이밍과 문서화 사소한 개선 제안MAX_PROBABILITY=100, COMMON_THRESHOLD=85, RARE_THRESHOLD=95는 명확합니다. 주석에 “1
85=COMMON, 8695=RARE, 96~100=SPECIAL” 한 줄 예시를 남겨두면 유지보수자가 빠르게 이해할 수 있습니다.
39-43: 캐싱 고려(선택): 타입별 컬러 목록은 변동 적음타입별 컬러 목록은 자주 바뀌지 않는 정적 데이터에 가깝습니다. pickColor() 호출 빈도가 높다면,
getColorsByType에 @Cacheable(type) 캐시를 두거나, 애플리케이션 기동 시 메모리 로드하여 참조하는 방식으로 DB 왕복을 줄일 수 있습니다.Also applies to: 64-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/aloc/aloc/global/init/DevDataInitializer.java(2 hunks)src/main/java/com/aloc/aloc/profilebackgroundcolor/enums/ColorType.java(1 hunks)src/main/java/com/aloc/aloc/profilebackgroundcolor/service/ProfileBackgroundColorService.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/aloc/aloc/profilebackgroundcolor/enums/ColorType.java (3)
src/main/java/com/aloc/aloc/profilebackgroundcolor/entity/ProfileBackgroundColor.java (1)
Entity(15-36)src/main/java/com/aloc/aloc/user/dto/response/UserColorChangeResponseDto.java (1)
Data(10-28)src/main/java/com/aloc/aloc/profilebackgroundcolor/dto/response/ColorResponseDto.java (2)
Getter(8-47)of(35-46)
src/main/java/com/aloc/aloc/profilebackgroundcolor/service/ProfileBackgroundColorService.java (5)
src/main/java/com/aloc/aloc/profilebackgroundcolor/entity/ProfileBackgroundColor.java (1)
Entity(15-36)src/test/java/com/aloc/aloc/profilebackgroundcolor/service/ProfileBackgroundColorServiceTest.java (2)
ExtendWith(22-70)Test(30-52)src/main/java/com/aloc/aloc/profilebackgroundcolor/repository/ProfileBackgroundColorRepository.java (1)
Repository(14-20)src/main/java/com/aloc/aloc/user/dto/response/UserColorChangeResponseDto.java (1)
Data(10-28)src/test/java/com/aloc/aloc/user/service/UserServiceTest.java (2)
Nested(64-130)Test(68-91)
src/main/java/com/aloc/aloc/global/init/DevDataInitializer.java (3)
src/main/java/com/aloc/aloc/profilebackgroundcolor/entity/ProfileBackgroundColor.java (1)
Entity(15-36)src/main/java/com/aloc/aloc/profilebackgroundcolor/repository/ProfileBackgroundColorRepository.java (1)
Repository(14-20)src/main/java/com/aloc/aloc/profilebackgroundcolor/dto/response/ColorResponseDto.java (1)
Getter(8-47)
🔇 Additional comments (6)
src/main/java/com/aloc/aloc/profilebackgroundcolor/enums/ColorType.java (1)
7-20: Enum 도입과 도메인 분리, 깔끔합니다문자열 상수 대신 Enum으로 분리해 전역에서 재사용하는 방향 좋습니다. 서비스/초기 데이터 모두에서 일관되게 사용할 수 있어 유지보수성 향상에 크게 기여합니다.
src/main/java/com/aloc/aloc/global/init/DevDataInitializer.java (2)
34-52: Enum 값 사용으로 상수 문자열 제거: 유지보수성 향상하드코딩된 "common/rare/special" 제거하고 ColorType.getValue()로 치환한 점 좋습니다. 저장소가 String 타입을 받는 제약을 고려한 실용적 접근입니다.
Also applies to: 54-61, 63-71, 72-80, 81-89, 90-98, 99-107, 126-139
31-141: 데이터 품질 점검 결과: 이상 없음
- 중복 name(@id) 없음
- color4/color5 필드에 빈 문자열 없음
- 타입 분포(COMMON: 10, RARE: 10, SPECIAL: 2)가 코드 의도와 일치
src/main/java/com/aloc/aloc/profilebackgroundcolor/service/ProfileBackgroundColorService.java (3)
39-43: 단계 분리로 가독성과 테스트 용이성이 향상되었습니다pickColor()를 세 단계로 분리한 구조(타입 결정 → 타입별 조회 → 무작위 선택)가 명확하고 유지보수에 유리합니다. 경계값(85/95) 처리도 off-by-one 없이 정확합니다.
Also applies to: 45-58
85-85: toList 사용 적절Stream 마무리에 toList() 사용으로 불변 리스트 반환되는 점 좋습니다. DTO 변환도 간결합니다.
30-34: 예외 타입/메시지 적절존재하지 않는 이름 조회 시 IllegalArgumentException 사용과 구체 메시지 모두 적절합니다. 테스트도 이에 맞춰 잘 구성되어 있습니다.
1 participant
📌 PR 제목
🔧 리팩토링 내용
ColorType enum 분리: 전역에서 사용 가능하도록 별도 파일로 분리
메서드 분리: pickColor() 메서드를 단일 책임 원칙에 따라 3개 메서드로 분리
determineColorTypeByProbability(): 확률 기반 타입 결정
getColorsByType(): 타입별 색상 조회 + 예외 처리
selectRandomColor(): 랜덤 색상 선택
상수화: 매직 넘버를 의미있는 상수로 변경 (COMMON_THRESHOLD, RARE_THRESHOLD)
변수명 개선: 더 명확한 변수명 사용 (randomNumberGenerator, randomNumber)
예외 처리 강화: 빈 색상 리스트에 대한 예외 처리 추가
성능 최적화: Random 인스턴스를 클래스 레벨에서 재사용
🏗️ 구조 개선
Before: 20줄의 복잡한 pickColor() 메서드
After: 3줄의 간결한 pickColor() 메서드 + 분리된 private 메서드들
🔍 체크리스트
📸 스크린샷 (UI 변경이 있다면)
🔗 관련 이슈
Summary by CodeRabbit