1주차 미션/ 서버 1조 박지원 #4
Conversation
| public static final int LEFT_DRAW_CONSTANT = -1; | ||
| public static final int RIGHT_DRAW_CONSTANT = 1; | ||
| public static final int NOTHING = 0; | ||
| public static final int COL_BLOCK_SIZE = 1; | ||
| public static final int START_INDEX = 0; |
There was a problem hiding this comment.
final 상수를 통해 상수도 관리하는 것도 좋지만, 가독성이 떨어지는 장점이 있는것 같아요!
final 상수 대신 코드가 단순해지고, 가독성이 좋은 enum 클래스를 사용해보는 것은 어떨까요?
| package game.component.util; | ||
|
|
||
| public enum PipeStatus { | ||
| Left(PipeConstant.LEFT_DRAW_CONSTANT), Right(PipeConstant.RIGHT_DRAW_CONSTANT), Nothing(PipeConstant.NOTHING); |
There was a problem hiding this comment.
PipeStatus 에서 LEFT_DRAW_CONSTANT, RIGHT_DRAW_CONSTANT, NOTHING 상수를 선언해서 관리하는 것이 나을것 같아요!
| public class CustomException extends RuntimeException { | ||
| private final ErrorCode errorCode; | ||
|
|
||
| public CustomException(ErrorCode errorCode) { | ||
| super(errorCode.getMessage()); | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public ErrorCode getErrorCode() { | ||
| return errorCode; | ||
| } | ||
| } |
| validateBallPosition(position[0],position[1]); | ||
| ball.initializePosition(position[0],position[1]); |
There was a problem hiding this comment.
validateBallPosition, initializePosition 메서드가 runWithPrint 에서 중복되는 것 같아요! 따로 이 부분을 private 메서드로 빼는 것도 좋을 것 같습니다.
| } | ||
| } | ||
| private void validateBallPosition(int row,int col){ | ||
| if(row<0||row>=rows.length) |
There was a problem hiding this comment.
if 문안에 row<0|| row>=rows.length를 따로 private 메서드로 빼서 이름을 지정하면, 더 가독성이 좋을 것 같아요!
| private int row; | ||
| private int col; |
There was a problem hiding this comment.
row 와 col 필드가 1 이상인 정수 조건을 가져야 하므로 예외 처리 메서드도 필요할 것 같네요!
| private PipeStatus pipeStatus; | ||
|
|
||
| public Pipe(){ | ||
| this.pipeStatus=PipeStatus.Nothing; | ||
| } |
| rows[i] = new Row(numberOfPerson); | ||
| } | ||
| } | ||
| public Ladder(int numberOfRows, int numberOfPerson,int randomCount) { |
There was a problem hiding this comment.
메서드의 이름을 Ladder 보다 랜덤 카운트를 받는다는 의미를 가진 이름으로 설정하는게 나아보여요! LadderWithRandom 이런 식으로요!
|
|
||
| public class LadderConstant { | ||
|
|
||
| public static final int ROW_BLOCK_SIZE = 1; |
There was a problem hiding this comment.
상수가 하나뿐이라, 단순히 클래스로 빼는것보다 Ladder 클래스에서 선언하는것도 좋을 것 같아요!
| try { | ||
| validateDrawLinePosition(position); | ||
| } | ||
| catch(Exception e){ | ||
| return false; | ||
| } |
안녕하세요 서버 1조 박지원(david-parkk)입니다.
설계의 중요성을 망각하고 항상 코드를 막쓰는 것이 습관화되어 객체지향적으로 개발하는데 많은 어려움이 있었습니다.
제 바람과 달리 스파게티 코드가 만들어져 코드리뷰하시는 파트장님께 미리 사과 드리겠습니다. 😢
제가 중점적으로 생각하고 진행한 내용에 대해 설명드리겠습니다.
1. 매직 넘버, 리터럴 관리
2. 원시값 포장
int [] rows; 을 int 형에서 Pipe 클래스로 대체하였습니다.3. 인터페이스화
4. 패키지화
5. 정적 팩터리 메소드
구현하면서 가장 크게 느낀점으로는 단순히 리펙토링을 하는 것보다 기능을 추가하면서 리펙토링하는 것이 더 좋은 작업 능률을 가졌다는 점과 객체지향개발이 정말 어렵다는 것을 느꼈습니다