2주차 미션 / 서버 1조 김가현 #41
Conversation
Walkthrough새로운 사다리 도메인 구조와 실행 흐름이 추가되었고, 게임 생성용 팩토리, 러너, 크리에이터(기본/랜덤) 및 유틸리티 클래스가 도입되었습니다. 사다리 출력 로직이 Row 기반으로 재구성되었으며 테스트 의존성(AssertJ)이 추가되었습니다. IDE 설정 파일들이 생성·수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester as Test/Client
participant Factory as LadderGameFactory
participant Creator as LadderCreator (Default/Random)
participant Game as LadderGame
participant Runner as LadderRunner
Tester->>Factory: create...(rowSize, numberOfPerson)
Factory->>Creator: new ...Creator()\ncreate(rowSize, numberOfPerson)
Creator-->>Factory: Row[]
alt Random creator
Factory->>Creator: drawLine(Position or RNG loop)
end
Factory-->>Tester: LadderGame(Game)
Tester->>Game: drawLine(Position) [옵션]
Game->>Creator: drawLine(Position)
Tester->>Game: run(startCol)
Game->>Runner: new LadderRunner(Creator.getRows())
Runner->>Runner: validate start index
loop for each Row
Runner->>Runner: move via Row.moveCoord()
Runner->>Runner: update Position
end
Runner-->>Game: finalCol
Game-->>Tester: finalCol
sequenceDiagram
autonumber
participant Creator as DefaultLadderCreator
participant Row as Row[i]
Creator->>Row: canDrawLine()
alt can draw
Creator->>Creator: Position.isValid(maxRow,maxCol)
alt valid
Creator->>Row: drawRow(col)
Creator->>Row: drawRow(col+1)
else invalid
Creator-->>Creator: throw INVALID_DRAW_POSITION
end
else cannot draw
Creator-->>Creator: throw INVALID_DRAW_POSITION
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (30)
.idea/.name (1)
1-1: IDE 전용 파일은 VCS에서 제외 권장.idea/.name은 팀원별 환경차로 잦은 충돌을 유발합니다. .idea 디렉터리는 .gitignore에 추가해 소스 변경과 분리하는 것을 권장합니다.
build.gradle (1)
15-15: JUnit BOM 버전 상향 제안 (5.13.3 권장)AssertJ 3.26.3 사용과 호환성, 최신 Jupiter 기능을 위해 junit-bom을 5.13.3으로 올리는 것을 권장합니다.
다음 변경을 제안합니다 (기능 영향 없음):
- testImplementation platform('org.junit:junit-bom:5.10.0') + testImplementation platform('org.junit:junit-bom:5.13.3')(참고: 5.13.x의 개선점과 호환성은 제공된 러닝 노트 요약을 참조하세요.)
Based on learnings.idea/misc.xml (1)
7-9: IDE 설정 파일 커밋 최소화 권장ProjectRootManager 속성 변경은 개인 환경 차에 민감합니다. .idea 전체를 .gitignore에 포함해 팀 협업 시 불필요한 변경/충돌을 줄이길 권장합니다.
.idea/gradle.xml (1)
3-16: IDE Gradle 설정은 VCS 제외 권장gradleJvm 등 개발자 환경 의존 속성은 리포지토리에서 관리하지 않는 편이 안전합니다. .idea/를 .gitignore에 추가하는 것을 권장합니다.
.idea/uiDesigner.xml (1)
1-124: UI Designer 팔레트 설정은 리포지토리 제외 권장해당 파일은 IDE 전용 메타데이터입니다. 팀 협업에서 소스와 무관한 변경을 유발하므로 .idea/ 디렉터리 통째로 .gitignore 처리 권장합니다.
src/main/java/ExceptionMessage.java (1)
2-8: 주석 처리된 enum 상수 정리 및 메시지 언어 일관화주석 처리된 상수는 혼란을 야기하므로 제거하거나 이슈/문서로 이전을 권장합니다. 또한 메시지 언어(영문/국문)를 프로젝트 전반에서 일관되게 유지해 주세요.
적용 예:
-// INVALID_LADDER_POSITION("사다리 위치는 1이상 자연수입니다."), -// INVALID_LADDER_NUMBER("사다리의 행과 열은 2 이상이어야 합니다."), +// (필요 시 향후 추가) @@ -// INVALID_NATURAL_NUMBER("자연수가 아닙니다."); +// (필요 시 향후 추가)src/main/java/Row.java (3)
2-4: 필드 불변성/캡슐화 강화
- row는 참조가 바뀌지 않으므로 final 권장.
- rowSize는 외부 변경 필요 없어 보이며 private final 권장.
- private int[] row; - public final int rowSize; + private final int[] row; + private final int rowSize;추가로 필요 시 접근자 제공:
public int getRowSize() { return rowSize; }
19-29: 표준 출력 의존 제거 제안showRow/showTotalRow는 System.out에 직접 출력합니다. 테스트/재사용성을 위해 문자열 반환(toString) 또는 로깅 사용을 권장합니다.
31-56: 모델 표현 명확화: 컬럼 수 vs 엣지 수 네이밍 통일
- 생성자 인자
size는 참가자(열) 수를 의미하므로rowSize→columnCount, 배열row→edges로 필드명 변경edges.length == columnCount, 실제 그리기 가능한 엣지 인덱스는0..columnCount-2임을 주석과 검증 로직에 반영src/test/java/LadderTest.java (1)
1-39: 주석 처리된 테스트 정리전체가 주석 처리되어 죽은 코드 상태입니다. 불필요하다면 삭제, 필요하다면 최신 API에 맞춰 복원해 주세요. 새로 추가된 테스트가 충분하다면 본 파일 제거를 권장합니다.
src/test/java/DefaultLadderGameTest.java (2)
7-13: 테스트 대상 생성은 @beforeeach에서 초기화하세요테스트 인스턴스 수명과 의도를 명확히 하기 위해 필드 초기화 대신 setUp에서 생성하는 편이 깔끔합니다.
public class DefaultLadderGameTest { - LadderGame ladderGame = LadderGameFactory.createDefaultLadderGame(7, 7); + LadderGame ladderGame; // LadderCreator ladderCreator = ladderGame.getLadderCreator(); @BeforeEach void setUp() { + ladderGame = LadderGameFactory.createDefaultLadderGame(7, 7); ladderGame.drawLine(new Position(0,0)); ladderGame.drawLine(new Position(1,1));
9-21: 주석 처리된 죽은 코드 제거읽기성 저하 및 유지보수 비용만 증가시키는 주석 블록입니다. 삭제 권장합니다.
-// LadderCreator ladderCreator = ladderGame.getLadderCreator(); @@ -// ladderCreator.drawLine(new Position(0, 0)); - -// ladderCreator.drawLine(new Position(1, 1)); - -// ladderCreator.drawLine(new Position(2, 5)); - -// ladderCreator.drawLine(new Position(5, 2)); @@ -// LadderRunner ladderRunner = new LadderRunner(ladderCreator.getRows()); - -// assertEquals(2, ladderRunner.run(1));Also applies to: 28-30
src/test/java/RandomLadderCreateTest.java (2)
4-4: AssertJ 정적 임포트 정정권장되는 임포트는 Assertions.assertThat 입니다. ForClassTypes는 비일반적입니다.
Based on learnings
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.assertj.core.api.Assertions.assertThat;
16-22: 출력 테스트에 단언 추가콘솔 출력만으로는 회귀 탐지가 어렵습니다. 최소 크기/널 여부를 검증하세요.
@Test @DisplayName("랜덤 사다리 출력") void drawAndShowLadder(){ ladderGame = LadderGameFactory.createRandomLadderGame(7, 7); Ladder ladder = new Ladder(ladderGame.getRows()); + assertThat(ladderGame.getRows()).hasSize(7).doesNotContainNull(); ladder.showTotalLadder(); }src/test/java/RandomLadderGameTest.java (1)
16-22: println 대신 유효 범위 단언으로 교체무작위 결과라도 결과 인덱스의 유효 범위를 검증해야 테스트로서 가치가 있습니다.
@Test @DisplayName("사다리 게임 정상 작동 테스트") void testGame() { - System.out.println("resultCol: "+ladderGame.run(1)); + int result = ladderGame.run(1); + org.junit.jupiter.api.Assertions.assertTrue(result >= 0 && result < 7, + "결과 컬럼은 0 이상 7 미만이어야 합니다."); // LadderRunner ladderRunner = new LadderRunner(ladderCreator.getRows()); // assertEquals(2, ladderRunner.run(1)); }src/main/java/LadderCreator.java (2)
6-34: 인터페이스에 남은 대량의 주석/구현 잔재 제거인터페이스 파일에 구현 주석이 대량 포함되어 있어 가독성을 크게 해칩니다. 과감히 제거하세요.
-// private final Row[] rows; -// -// -// //constructor -// //Todo: int type wrapper로 감싸기 -// public LadderCreator(int rowSize, int numberOfPerson) { -// -// rows = new Row[rowSize]; -// -// for (int i = 0; i<rowSize;i++) { -// -// rows[i] = new Row(numberOfPerson); -// -// } -// } -// -// -// public Row[] getRows() { -// -// return rows; -// -// } -// -// public void drawLine(Position position) { -// -// int positionRow = position.getRow(); -// -// //하나의 row(높이)에 2개 이상의 사다리 허용x -// -// if (!rows[positionRow].canDrawLine()) //입력받은 idx번째 row를 들고와서 검사 -// throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); -// -// //입력 받은 col과 col+1 에 가로줄 연결 -> 입력되는 col은 0~numberOfPerson-2까지만 허용 -// //각 position 이 유효한지 직접 검사함 -// if(position.isValid(rows.length-1, rows[0].rowSize-2)) { -// rows[positionRow].drawRow(position.getCol()); -// rows[positionRow].drawRow(position.getCol()+1); -// } -// }
2-5: 용어와 시그니처 정돈 제안
- numberOfPerson → numberOfPersons 또는 colSize가 더 명확합니다.
- create는 반환값 없이 내부 상태 초기화만 하도록 바꾸는 설계가 일관됩니다(현재 팩토리에서 반환값을 사용하지 않음).
이 변경은 구현 클래스 전체에 파급되므로 후속 PR에서 일괄 반영을 권장합니다.
src/test/java/DefaultLadderCreateTest.java (4)
4-4: AssertJ 정적 임포트 정정일반적인 assertThat 임포트를 사용하세요.
Based on learnings
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.assertj.core.api.Assertions.assertThat;
12-14: 클래스 의도와 일치하는 팩토리 사용클래스명이 DefaultLadderCreateTest인 만큼 default 팩토리를 사용하면 혼란이 줄어듭니다.
- ladderGame = LadderGameFactory.createRandomLadderGame(7, 7); + ladderGame = LadderGameFactory.createDefaultLadderGame(7, 7);
17-30: 주석 처리된 보일러플레이트 정리불필요한 주석 블록은 제거하세요.
-// LadderCreator ladderCreator; -// Ladder ladder; -// -// @BeforeEach -// void setUp() { -// ladderCreator = new LadderCreator(7, 7); -// } -// -// @Test -// @DisplayName("사다리 생성 확인") -// void createTest(){ -// assertThat(ladderCreator).isNotNull(); -// }
31-44: 출력 테스트에 검증 추가연결 후 최소한의 구조적 불변식을 확인하세요.
@Test @DisplayName("사다리 연결") void drawAndShowLadder(){ //LadderCreator 를 상속받은 RandomLadderCreator type을 반환함 //createDefault~는 defaultCreator 를 만들어주는 LadderGame 을 반환함 ladderGame = LadderGameFactory.createDefaultLadderGame(7, 7); // LadderCreator ladderCreator = ladderGame.getLadderCreator(); ladderGame.drawLine(new Position(1, 2)); ladderGame.drawLine(new Position(2, 3)); ladderGame.drawLine(new Position(4, 0)); ladderGame.drawLine(new Position(5, 5)); + assertThat(ladderGame.getRows()).hasSize(7).doesNotContainNull(); Ladder ladder = new Ladder(ladderGame.getRows()); ladder.showTotalLadder(); }src/main/java/LadderGame.java (1)
17-27: run() 내부 중복 접근/불필요 주석 제거 및 초기화 검증rows를 한 번만 조회하고, 초기화 검증을 추가하세요. 남아있는 주석은 혼란만 가중합니다.
public int run(int startColNumber) { // 1. 사다리 오기 -// Row[] rows = ladderCreator.create(position.getRow(), position.getCol()); Row[] rows = ladderCreator.getRows(); + if (rows == null || rows.length == 0) { + throw new IllegalStateException("사다리가 초기화되지 않았습니다."); + } // 2. runner 에게 사다리 전달 - LadderRunner ladderRunner = new LadderRunner(ladderCreator.getRows()); + LadderRunner ladderRunner = new LadderRunner(rows); //3. runner를 통해 게임 진행 return ladderRunner.run(startColNumber); }src/main/java/LadderRunner.java (1)
17-23: 콘솔 출력 노이즈 최소화showLadder 호출은 운영/테스트 로그를 과도하게 만듭니다. 로깅 레벨로 전환하거나 플래그로 토글 가능하게 분리하는 것을 권장합니다.
src/main/java/LadderGameFactory.java (1)
5-6: RandomLadderCreator.drawLine에 Position(0,0) 전달 의미 불명확.현재 Random 구현은 position 파라미터를 사용하지 않습니다. API 오해를 줄이기 위해 시그니처 정리(예: drawLines()) 또는 주석으로 의도를 명시하세요.
src/main/java/DefaultLadderCreator.java (2)
36-36: Row 내부 필드 직접 접근(rowSize) 지양.캡슐화 위반입니다. Row에 접근자(getColSize 등)를 추가해 사용하세요.
13-21: 주석 처리된 생성자 제거 권장.불필요한 주석 코드는 가독성을 해칩니다. 필요 시 Git 히스토리로 대체 가능합니다.
적용 예:
-//constructor -// //Todo: int type wrapper로 감싸기 -// -// public DefaultLadderCreator(int rowSize, int numberOfPerson) { -// -// rows = new Row[rowSize]; -// -// for (int i = 0; i<rowSize;i++) { -// -// rows[i] = new Row(numberOfPerson); -// -// } -// -// }src/main/java/RandomLadderCreator.java (2)
1-9: Random 대신 ThreadLocalRandom 사용 및 시드 제어 제거.성능/편의 측면에서 ThreadLocalRandom 권장. 수동 seed 설정은 재현성도 보장 못합니다.
아래처럼 교체하세요:
-import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; @@ -//난수 생성 변수 -Random random = new Random(); +// 난수 생성은 ThreadLocalRandom.current() 사용
21-21: drawLine(Position)의 position 파라미터 미사용.인터페이스/구현 의미 불일치입니다. 시그니처 정리 또는 Javadoc/주석으로 의도를 명확히 하세요.
src/main/java/Ladder.java (2)
7-9: rows null 방지 가드 추가.생성자에서 null 전달 시 NPE 발생. 명확한 예외로 방지하세요.
아래처럼 적용:
public Ladder(Row[] rows) { - this.rows = rows; + java.util.Objects.requireNonNull(rows, "rows must not be null"); + this.rows = rows; }
28-35: 매직 넘버(-1) 상수화.의미가 드러나지 않습니다. 상수로 치환하세요.
예시:
public class Ladder { private final Row[] rows; + private static final int NO_HIGHLIGHT = -1; @@ - //todo: magic num 없애기 - rows[i].showRow(-1); + rows[i].showRow(NO_HIGHLIGHT); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.idea/.name(1 hunks).idea/gradle.xml(1 hunks).idea/misc.xml(1 hunks).idea/uiDesigner.xml(1 hunks)build.gradle(1 hunks)src/main/java/DefaultLadderCreator.java(1 hunks)src/main/java/ExceptionMessage.java(1 hunks)src/main/java/GreaterThanOne.java(1 hunks)src/main/java/Ladder.java(1 hunks)src/main/java/LadderCreator.java(1 hunks)src/main/java/LadderGame.java(1 hunks)src/main/java/LadderGameFactory.java(1 hunks)src/main/java/LadderRunner.java(1 hunks)src/main/java/Position.java(1 hunks)src/main/java/RandomLadderCreator.java(1 hunks)src/main/java/Row.java(1 hunks)src/test/java/DefaultLadderCreateTest.java(1 hunks)src/test/java/DefaultLadderGameTest.java(1 hunks)src/test/java/LadderTest.java(1 hunks)src/test/java/RandomLadderCreateTest.java(1 hunks)src/test/java/RandomLadderGameTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-23T00:10:38.223Z
Learnt from: ParkSeongGeun
PR: Konkuk-KUIT/KUIT6_Server_Ladder#29
File: src/main/java/AppConfig.java:28-32
Timestamp: 2025-09-23T00:10:38.223Z
Learning: AppConfig.java에서 GameRunner를 LadderRunner로 캐스팅할 때, 사용자는 과도한 안전성 검증이 필요한지 의문을 표했음. 학습 단계에서는 간단한 instanceof 체크 정도가 적절한 수준의 안전성을 제공한다고 안내함.
Applied to files:
src/main/java/LadderGame.javasrc/main/java/LadderRunner.javasrc/test/java/LadderTest.javasrc/test/java/RandomLadderGameTest.java
🧬 Code graph analysis (4)
src/test/java/RandomLadderCreateTest.java (1)
src/main/java/LadderGameFactory.java (1)
LadderGameFactory(1-15)
src/test/java/DefaultLadderGameTest.java (1)
src/main/java/LadderGameFactory.java (1)
LadderGameFactory(1-15)
src/test/java/DefaultLadderCreateTest.java (1)
src/main/java/LadderGameFactory.java (1)
LadderGameFactory(1-15)
src/test/java/RandomLadderGameTest.java (1)
src/main/java/LadderGameFactory.java (1)
LadderGameFactory(1-15)
🔇 Additional comments (1)
src/main/java/ExceptionMessage.java (1)
11-17: LGTM: 불변 메시지와 접근자 구성 적절message 필드 불변성 유지와 getMessage 제공이 명확합니다.
| public Row[] create(int rowSize, int numberOfPerson) { | ||
| rows = new Row[rowSize]; | ||
| for (int i = 0; i<rowSize;i++) { | ||
| rows[i] = new Row(numberOfPerson); | ||
| } | ||
| return rows; | ||
| } |
There was a problem hiding this comment.
create(): 파라미터 유효성 검증 누락.
rowSize <= 0 또는 numberOfPerson < 2면 이후 로직에서 예외 발생(인덱스/난수 범위 등). 선제 체크가 필요합니다.
적용 예:
@Override
public Row[] create(int rowSize, int numberOfPerson) {
+ if (rowSize <= 0) throw new IllegalArgumentException("rowSize must be > 0");
+ if (numberOfPerson < 2) throw new IllegalArgumentException("numberOfPerson must be >= 2");
rows = new Row[rowSize];
for (int i = 0; i<rowSize;i++) {
rows[i] = new Row(numberOfPerson);
}
return rows;
}🤖 Prompt for AI Agents
In src/main/java/DefaultLadderCreator.java around lines 5 to 11, the create(int
rowSize, int numberOfPerson) method lacks parameter validation causing potential
IndexOutOfBounds or random-range errors when rowSize <= 0 or numberOfPerson < 2;
add upfront checks that throw IllegalArgumentException with clear messages if
rowSize <= 0 or numberOfPerson < 2, e.g., validate both parameters before
allocating the rows array and constructing Row instances so the method fails
fast with a descriptive error.
| public void drawLine(Position position) { | ||
| int positionRow = position.getRow(); | ||
| //하나의 row(높이)에 2개 이상의 사다리 허용x | ||
| if (!rows[positionRow].canDrawLine()) //입력받은 idx번째 row를 들고와서 검사 | ||
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | ||
|
|
||
| //입력 받은 col과 col+1 에 가로줄 연결 -> 입력되는 col은 0~numberOfPerson-2까지만 허용 | ||
| //각 position 이 유효한지 직접 검사함 | ||
| if(position.isValid(rows.length-1, rows[0].rowSize-2)) { | ||
| rows[positionRow].drawRow(position.getCol()); | ||
| rows[positionRow].drawRow(position.getCol()+1); | ||
| } | ||
| } |
There was a problem hiding this comment.
drawLine(): 유효성 검사 순서 오류로 인한 AIOOBE/NPE 가능.
position 유효성 확인 전에 rows[positionRow] 접근으로 예외 위험. 또한 invalid일 때 조용히 무시하지 말고 예외를 던지세요.
아래처럼 순서/검증을 정리하세요:
@Override
public void drawLine(Position position) {
- int positionRow = position.getRow();
- //하나의 row(높이)에 2개 이상의 사다리 허용x
- if (!rows[positionRow].canDrawLine()) //입력받은 idx번째 row를 들고와서 검사
- throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage());
-
- //입력 받은 col과 col+1 에 가로줄 연결 -> 입력되는 col은 0~numberOfPerson-2까지만 허용
- //각 position 이 유효한지 직접 검사함
- if(position.isValid(rows.length-1, rows[0].rowSize-2)) {
- rows[positionRow].drawRow(position.getCol());
- rows[positionRow].drawRow(position.getCol()+1);
- }
+ if (rows == null) throw new IllegalStateException("create(...) must be called before drawLine(...)");
+ if (position == null) throw new IllegalArgumentException("position must not be null");
+ int maxRow = rows.length - 1;
+ int maxStartCol = rows[0].rowSize - 2; // col, col+1을 그리기 위한 시작 col 상한
+ if (!position.isValid(maxRow, maxStartCol)) {
+ throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage());
+ }
+ int positionRow = position.getRow();
+ // 하나의 row(높이)에 2개 이상의 사다리 허용 x
+ if (!rows[positionRow].canDrawLine()) {
+ throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage());
+ }
+ rows[positionRow].drawRow(position.getCol());
+ rows[positionRow].drawRow(position.getCol() + 1);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void drawLine(Position position) { | |
| int positionRow = position.getRow(); | |
| //하나의 row(높이)에 2개 이상의 사다리 허용x | |
| if (!rows[positionRow].canDrawLine()) //입력받은 idx번째 row를 들고와서 검사 | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | |
| //입력 받은 col과 col+1 에 가로줄 연결 -> 입력되는 col은 0~numberOfPerson-2까지만 허용 | |
| //각 position 이 유효한지 직접 검사함 | |
| if(position.isValid(rows.length-1, rows[0].rowSize-2)) { | |
| rows[positionRow].drawRow(position.getCol()); | |
| rows[positionRow].drawRow(position.getCol()+1); | |
| } | |
| } | |
| @Override | |
| public void drawLine(Position position) { | |
| if (rows == null) { | |
| throw new IllegalStateException("create(...) must be called before drawLine(...)"); | |
| } | |
| if (position == null) { | |
| throw new IllegalArgumentException("position must not be null"); | |
| } | |
| int maxRow = rows.length - 1; | |
| int maxStartCol = rows[0].rowSize - 2; // col, col+1을 그리기 위한 시작 col 상한 | |
| if (!position.isValid(maxRow, maxStartCol)) { | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | |
| } | |
| int positionRow = position.getRow(); | |
| // 하나의 row(높이)에 2개 이상의 사다리 허용 x | |
| if (!rows[positionRow].canDrawLine()) { | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | |
| } | |
| rows[positionRow].drawRow(position.getCol()); | |
| rows[positionRow].drawRow(position.getCol() + 1); | |
| } |
| private void validate(int number) { | ||
| if(number<1) | ||
| throw new IllegalArgumentException("value must be at least 1"); | ||
| } |
There was a problem hiding this comment.
이름과 검증 조건 불일치로 인해 1을 허용하는 버그가 발생합니다.
클래스명이 GreaterThanOne이라서 불변식이 “1 초과”임을 기대하지만, 현재 검증은 number >= 1만 요구합니다. 이 상태로는 1이 통과해 이후 로직에서 “1 초과”를 가정하는 부분이 깨질 수 있습니다. 조건을 <= 1로 바꾸고 예외 메시지도 함께 조정해 주세요.
- private void validate(int number) {
- if(number<1)
- throw new IllegalArgumentException("value must be at least 1");
- }
+ private void validate(int number) {
+ if (number <= 1) {
+ throw new IllegalArgumentException("value must be greater than 1");
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void validate(int number) { | |
| if(number<1) | |
| throw new IllegalArgumentException("value must be at least 1"); | |
| } | |
| private void validate(int number) { | |
| if (number <= 1) { | |
| throw new IllegalArgumentException("value must be greater than 1"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/GreaterThanOne.java around lines 9 to 12, the validate method
currently allows 1 because it checks number < 1; change the guard to reject 1 by
using number <= 1 and update the exception message to reflect the invariant
(e.g., "value must be greater than 1") so the method enforces the class name
expectation.
| @@ -1,8 +1,36 @@ | |||
| import java.sql.SQLOutput; | |||
There was a problem hiding this comment.
불필요/오류 가능성이 있는 import 제거.
java.sql.SQLOutput는 사용되지 않습니다.
적용 예:
-import java.sql.SQLOutput;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import java.sql.SQLOutput; |
🤖 Prompt for AI Agents
In src/main/java/Ladder.java around lines 1 to 1, remove the unused import
statement "import java.sql.SQLOutput;" as it is unnecessary and may cause
confusion; update the imports accordingly (remove the line), save the file, and
recompile or run linting to ensure no other unused imports remain.
| public void showLadder(String message, Position pos) { | ||
| System.out.println(message); | ||
| for (int i = 0; i < rows.length; i++) { | ||
| orderShowRow(i, pos.getRow(), pos.getCol()); | ||
| System.out.println(); | ||
| } | ||
| System.out.println(); | ||
| } |
There was a problem hiding this comment.
pos null 처리 필요.
pos가 null이면 NPE. 유효성 체크 또는 메시지로 방지하세요.
간단 예:
public void showLadder(String message, Position pos) {
System.out.println(message);
+ if (pos == null) throw new IllegalArgumentException("pos must not be null");
for (int i = 0; i < rows.length; i++) {
orderShowRow(i, pos.getRow(), pos.getCol());
System.out.println();
}
System.out.println();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void showLadder(String message, Position pos) { | |
| System.out.println(message); | |
| for (int i = 0; i < rows.length; i++) { | |
| orderShowRow(i, pos.getRow(), pos.getCol()); | |
| System.out.println(); | |
| } | |
| System.out.println(); | |
| } | |
| public void showLadder(String message, Position pos) { | |
| System.out.println(message); | |
| if (pos == null) throw new IllegalArgumentException("pos must not be null"); | |
| for (int i = 0; i < rows.length; i++) { | |
| orderShowRow(i, pos.getRow(), pos.getCol()); | |
| System.out.println(); | |
| } | |
| System.out.println(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/Ladder.java around lines 12 to 19, the method showLadder(String
message, Position pos) dereferences pos without checking for null which can
cause an NPE; add a null check at the start (e.g., if (pos == null) {
System.out.println("Position is null"); return; } or throw
IllegalArgumentException) so the method either fails fast with a clear message
or handles the null case gracefully before using pos.getRow()/pos.getCol().
| private boolean validate(int positionRow, int positionCol) { | ||
| if(!rows[positionRow].isLineOccupied(positionCol)) | ||
| return true; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
validate(): 인접 셀 및 범위 검증 누락.
두 칸(col, col+1) 모두 비어있는지, 시작 col 범위가 유효한지 확인해야 합니다.
수정 예:
private boolean validate(int positionRow, int positionCol) {
- if(!rows[positionRow].isLineOccupied(positionCol))
- return true;
- return false;
+ int maxStartCol = rows[0].rowSize - 2;
+ if (positionCol < 0 || positionCol > maxStartCol) return false;
+ return !rows[positionRow].isLineOccupied(positionCol)
+ && !rows[positionRow].isLineOccupied(positionCol + 1);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private boolean validate(int positionRow, int positionCol) { | |
| if(!rows[positionRow].isLineOccupied(positionCol)) | |
| return true; | |
| return false; | |
| } | |
| private boolean validate(int positionRow, int positionCol) { | |
| int maxStartCol = rows[0].rowSize - 2; | |
| if (positionCol < 0 || positionCol > maxStartCol) return false; | |
| return !rows[positionRow].isLineOccupied(positionCol) | |
| && !rows[positionRow].isLineOccupied(positionCol + 1); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/RandomLadderCreator.java around lines 42 to 46, the validate
method currently only checks a single cell and can throw IndexOutOfBounds;
update it to first ensure positionRow is within rows array bounds and
positionCol and positionCol+1 are within the valid column range, then return
true only if both cells rows[positionRow].isLineOccupied(positionCol) and
rows[positionRow].isLineOccupied(positionCol+1) are false (i.e., both
unoccupied); otherwise return false — make these bounds checks defensive to
avoid exceptions.
| public boolean canDrawLine() { //사다리 중복 여부 검사 | ||
| for (int i : row) { | ||
| if (i == 1) | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
canDrawLine의 의미가 모호합니다
현재 구현은 행 내에 1이 하나라도 있으면 추가 선을 금지합니다. 일반적인 사다리 규칙은 "인접 금지"이지 "단 한 개만 허용"이 아닙니다. 사용처를 확인해 의미에 맞게 수정/제거를 권장합니다.
다음 스크립트로 사용 여부를 확인해 주세요:
🏁 Script executed:
#!/bin/bash
rg -n -C2 '\bcanDrawLine\s*\(' --type=javaLength of output: 1162
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/main/java/Row.javaLength of output: 1592
canDrawLine 메서드 의미 및 로직 수정 필요
현재 로직은 행에 1개 이상의 선이 있으면 모두 금지하지만, 사다리 규칙은 인접 금지입니다. DefaultLadderCreator.java:31 에서 호출 중인 canDrawLine을 인접 검사 로직(isLineOccupied)으로 대체하거나, 메서드 이름·로직을 실제 의도에 맞게 수정하세요.
🤖 Prompt for AI Agents
In src/main/java/Row.java around lines 11 to 17, the canDrawLine method
currently forbids drawing if any 1 exists in the row but ladder rules only
forbid adjacent lines; update the method (or rename to isLineOccupied and
replace callers) to check for adjacency instead: given an index to test, ensure
neither neighbor (index-1 and index+1) contains a 1 before allowing a draw, or
alternatively replace this method call at DefaultLadderCreator.java:31 with the
existing isLineOccupied logic there so the adjacency rule is enforced.
| public int moveCoord(int startColNumber) { | ||
| //Todo: rowsize-1 인가? | ||
| int pos = startColNumber; | ||
| if (pos <= rowSize - 2 && row[pos + 1] == 1) { //오른쪽 길이 있는 경우(1칸만 이동) | ||
| // System.out.println("right presColNumber: "+pos); | ||
| return ++pos; | ||
| } | ||
| if (pos >= 1 && row[pos - 1] == 1) { //왼쪽 길이 있는 경우 | ||
| // System.out.println("left presColNumber: "+pos); | ||
| return --pos; | ||
| } | ||
| return pos; | ||
| } |
There was a problem hiding this comment.
moveCoord 좌/우 이동 판정 오프바이원 수정 필요
우측 이동을 row[pos+1]==1로 검사하는 것은 인덱스 매핑상 비직관적이며 인접 판정과 불일치합니다. 일반적으로:
- 오른쪽 연결: row[pos]가 1이면 pos+1
- 왼쪽 연결: row[pos-1]가 1이면 pos-1
- public int moveCoord(int startColNumber) {
- //Todo: rowsize-1 인가?
- int pos = startColNumber;
- if (pos <= rowSize - 2 && row[pos + 1] == 1) { //오른쪽 길이 있는 경우(1칸만 이동)
- return ++pos;
- }
- if (pos >= 1 && row[pos - 1] == 1) { //왼쪽 길이 있는 경우
- return --pos;
- }
- return pos;
- }
+ public int moveCoord(int startColNumber) {
+ int pos = startColNumber;
+ if (pos < 0 || pos >= rowSize) {
+ throw new IllegalArgumentException(ExceptionMessage.INVALID_POSITION.getMessage());
+ }
+ // 오른쪽 연결: 현재 열에서 오른쪽으로 나가는 선
+ if (pos <= rowSize - 2 && row[pos] == 1) {
+ return pos + 1;
+ }
+ // 왼쪽 연결: 왼쪽 열에서 현재 열로 들어오는 선
+ if (pos >= 1 && row[pos - 1] == 1) {
+ return pos - 1;
+ }
+ return pos;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public int moveCoord(int startColNumber) { | |
| //Todo: rowsize-1 인가? | |
| int pos = startColNumber; | |
| if (pos <= rowSize - 2 && row[pos + 1] == 1) { //오른쪽 길이 있는 경우(1칸만 이동) | |
| // System.out.println("right presColNumber: "+pos); | |
| return ++pos; | |
| } | |
| if (pos >= 1 && row[pos - 1] == 1) { //왼쪽 길이 있는 경우 | |
| // System.out.println("left presColNumber: "+pos); | |
| return --pos; | |
| } | |
| return pos; | |
| } | |
| public int moveCoord(int startColNumber) { | |
| int pos = startColNumber; | |
| if (pos < 0 || pos >= rowSize) { | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_POSITION.getMessage()); | |
| } | |
| // 오른쪽 연결: 현재 열에서 오른쪽으로 나가는 선 | |
| if (pos <= rowSize - 2 && row[pos] == 1) { | |
| return pos + 1; | |
| } | |
| // 왼쪽 연결: 왼쪽 열에서 현재 열로 들어오는 선 | |
| if (pos >= 1 && row[pos - 1] == 1) { | |
| return pos - 1; | |
| } | |
| return pos; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/Row.java around lines 31-43, the right-movement check is
off-by-one: it inspects row[pos+1]==1 which wrongly tests the destination cell
instead of the current cell's right-connection; change the logic so the right
move checks row[pos]==1 and returns pos+1 (keeping the same upper bound check to
avoid IndexOutOfBounds), keep the left check as row[pos-1]==1 with pos>=1, and
update comments accordingly.
| public void drawRow(int colIdx) { //row에서 직접 사다리를 그림 | ||
| row[colIdx] = 1; | ||
| } |
There was a problem hiding this comment.
drawRow에 경계/충돌 검증 추가 필요
현재는 인덱스/인접 충돌 검증 없이 그려 ArrayIndexOutOfBounds 또는 규칙 위반 가능성이 있습니다.
- public void drawRow(int colIdx) { //row에서 직접 사다리를 그림
- row[colIdx] = 1;
- }
+ public void drawRow(int colIdx) { // row에서 직접 사다리를 그림
+ // 유효 위치: 0..rowSize-2 (열 사이 연결)
+ if (colIdx < 0 || colIdx > rowSize - 2) {
+ throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage());
+ }
+ if (isLineOccupied(colIdx)) {
+ throw new IllegalStateException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage());
+ }
+ row[colIdx] = 1;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void drawRow(int colIdx) { //row에서 직접 사다리를 그림 | |
| row[colIdx] = 1; | |
| } | |
| public void drawRow(int colIdx) { // row에서 직접 사다리를 그림 | |
| // 유효 위치: 0..rowSize-2 (열 사이 연결) | |
| if (colIdx < 0 || colIdx > rowSize - 2) { | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | |
| } | |
| if (isLineOccupied(colIdx)) { | |
| throw new IllegalStateException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | |
| } | |
| row[colIdx] = 1; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/Row.java around lines 45-47, drawRow currently writes
row[colIdx]=1 without any bounds or adjacency checks; add validation to prevent
ArrayIndexOutOfBounds and forbid creating adjacent ladders: first verify colIdx
>= 0 && colIdx < row.length, then check that (colIdx-1 is out of range or
row[colIdx-1]==0) and (colIdx+1 is out of range or row[colIdx+1]==0); if
validations pass set row[colIdx]=1, otherwise throw an IllegalArgumentException
(or return a boolean) with a clear message indicating whether it was an
out-of-bounds or adjacency violation.
| //row가 겹쳐지지 않는지, 연속되지 않는 지 검사하는 메서드 | ||
| public boolean isLineOccupied(int positionCol) { | ||
| if(positionCol==0) | ||
| return row[positionCol+2]==1||row[0]==1; | ||
| if(positionCol==rowSize-2) | ||
| return row[positionCol-1]==1||row[positionCol]==1; | ||
| return row[positionCol-1]==1||row[positionCol+2]==1||row[positionCol]==1; | ||
| } |
There was a problem hiding this comment.
isLineOccupied의 +2 인덱스는 범위/논리 오류 가능
positionCol+2 접근은 작은 rowSize에서 곧바로 인덱스 에러를 유발합니다. 인접 충돌 검사는 (pos-1, pos, pos+1)만 확인即可입니다.
- public boolean isLineOccupied(int positionCol) {
- if(positionCol==0)
- return row[positionCol+2]==1||row[0]==1;
- if(positionCol==rowSize-2)
- return row[positionCol-1]==1||row[positionCol]==1;
- return row[positionCol-1]==1||row[positionCol+2]==1||row[positionCol]==1;
- }
+ public boolean isLineOccupied(int positionCol) {
+ // 유효 위치 벗어나면 점유된 것으로 간주
+ if (positionCol < 0 || positionCol > rowSize - 2) return true;
+ // 해당 위치에 이미 선이 있거나, 좌/우 인접 선과 충돌하는지 검사
+ if (row[positionCol] == 1) return true;
+ if (positionCol > 0 && row[positionCol - 1] == 1) return true;
+ if (positionCol < rowSize - 2 && row[positionCol + 1] == 1) return true;
+ return false;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/Row.java around lines 49 to 56, the method isLineOccupied uses
positionCol+2 which can cause IndexOutOfBounds on small rows and is logically
wrong; change the checks to only consider adjacent slots (positionCol-1,
positionCol, positionCol+1) and guard each access with bounds checks: for
positionCol==0 only check 0 and 1 (if present), for positionCol==rowSize-1 only
check rowSize-2 and rowSize-1 (if present), otherwise check positionCol-1,
positionCol and positionCol+1; implement using explicit index-range conditions
(or min/max) to avoid any out-of-range array access.
1 participant
Summary by CodeRabbit
신기능
리팩터
테스트
작업