Search
Duplicate

4. [클린 코드 with Java] 4단계 - 사다리(리팩터링)

링크
점수
⭐️⭐️⭐️⭐️
완료일
2023/06/08
상태
완료
유형
인강

기능 요구사항

기능 요구사항 3단계와 같다.

실행 결과

위 요구사항에 따라 4명의 사람을 위한 5개 높이 사다리를 만들 경우, 프로그램을 실행한 결과는 다음과 같다.
참여할 사람 이름을 입력하세요. (이름은 쉼표(,)로 구분하세요) pobi,honux,crong,jk 실행 결과를 입력하세요. (결과는 쉼표(,)로 구분하세요),5000,,3000 최대 사다리 높이는 몇 개인가요? 5 사다리 결과 pobi honux crong jk |-----| |-----| | |-----| | |-----| | | | |-----| | |-----| |-----|50003000 결과를 보고 싶은 사람은? pobi 실행 결과 꽝 결과를 보고 싶은 사람은? all 실행 결과 pobi : 꽝 honux : 3000 crong : 꽝 jk : 5000
Java
복사

피드백

리뷰어 : 이 모델의 역할은 lines 상태를 변경해주는 것 같아요. 객체 참조를 통해서 외부에서 받아오는 객체의 상태를 변경하다보면해당 객체가 어떻게 변경되는지 파악이 어려워질 것 같아요.또한, 실제로 LadderGameController 에서도 내부에 상태가 변경될 것을 예상하고lines 를 활용하고 있는 것 같기 때문에 결국 Ladder의 캡슐화가 깨지게 된 것이 아닌가 생각됩니다이미 알고 계실 수도 있지만내부의 데이터를 보호하고 참조값 공유를 끊어주기 위해 방어적 복사도 많이 이용하는데요. 방어적 복사란 혹시 이러한 부분을 개선해보시는 방향은 어떠신가요??
public void playGame() { IntStream.range(0, players.getPlayerCount()) .forEach(index -> lines.getConnectNumber(players.getPlayers().get(index))); .forEach(index -> lines.move(players.getPlayers().get(index))); }
Java
복사
방어적 복사를 사용하는 이유 : 객체의 취약점을 보완하기 위해 사용. (내부의 데이터를 보호하고 참조값 공유를 끊어주기 위해 사용)
생각정리 : 주소값으로 연결되 있는 건 알았지만, 방어적 복사를 통해서 데이터를 보호한다는 것은 처음 인지하였다. 알아두면, 보호 목적으로 잘 사용 될 것 같다.
public Ladder(Players players, Lines lines) { this.players = new Players(players.getPlayers()); this.lines = new Lines(lines.getLines()); } public void playGame() { players.move(lines); }
Java
복사
리뷰어 : 메소드의 역할에 대해 메소드명이 어색한 것 같아요  내부에서는 특별한 조건이 있지만 클라이언트 입장에서는 왼쪽으로 옮겨갈 것 이라고 생각될 것 같은데 혹시 이부분에 대해서 어떻게 생각하시나요 
public void moveLeft(List<Boolean> points) { if (points.get(point - 1)) { point += Direction.LEFT.value; } }
Java
복사
생각정리 : 왼쪽으로 움직이는 행위만 작동시키게 하면 되는데, if 까지 엮이면서 오해의 소지가 있다고 생각된다.
public void moveLeft() { point += LEFT; }
Java
복사
리뷰어 : 혹시 public enum 으로 분리해주신 이유가 있을까요? 내부에서만 사용되고 있고 결국 value 만 바로 꺼내서 사용하고 있기 때문에 장점이 없는 것 같은데 혹시 이 부분에 대해서 어떻게 생각하시나요??? 
public enum Direction { LEFT(-1), RIGHT(1); private final int value; Direction(int value) { this.value = value; } }
Java
복사
생각정리 : 아무런 동작을 하지 않는다면 정적변수로 선언해서 사용해도 되는데, 굳이 변경하는 이유를 물어본것 같다. enum의 함수에서 동작을 처리하도록 하거나, 정적변수를 사용해서 작성하자.
public class Player { private static final int LEFT = -1; private static final int RIGHT = 1;
Java
복사
리뷰어 : 내부에서 ConnectionStrategy을 직접 생성하고 사용하고 있기 때문에 인터페이스로 분리한 장점이 없는 것 같아요  이 객체에서 Random 요소를 분리하기 위해 외부에서 주입 받는 방법은 어떻게 생각하시나요??
IntStream.range(RANGE_START, rangeEnd) .forEach(count -> { if (count == ZERO || !points.get(count - 1)) { connectLine(() -> RANDOM.nextBoolean()); return; } connectLine(() -> false); }); } public void connectLine(ConnectionStrategy connectionStrategy) { if (connectionStrategy.isConnection()) { points.add(true); return; } points.add(false); }
Java
복사
생각정리 : 익명 클래스를 사용해서 외부에서 주입 받는 형식으로 코드를 작성하였는데, 정작 사용하는 곧은 내부이니, 장점이 나오지 않는 것 같다. 외부에서 사용해서 전달 해주는 방식으로 변경하고, 유연하게 대처할수 있게 구현하자.
public void connectLine(ConnectionStrategy connectionStrategy) { if (connectionStrategy.isConnection()) { points.add(true); return; } points.add(false); }
Java
복사
출처