2주차 미션 / 서버 1조 신종윤 #37
Conversation
sh1220
left a comment
sh1220
left a comment
There was a problem hiding this comment.
수고 많으셨습니다!
전반적으로 강의상의 코드와 다르게, 자신만의 코드를 짠느낌이 강해서 개성이 느껴졌던 것 같아요.
다만, 조합이나 객체 간의 관계에서 조금 다듬었으면 하는 것들이 있어서 간단히 남겼습니다.
추가로, Line같은 거를 보면 Ladder안에 rows와 line이 있고 다시 line이 rows를 가지고 있는 모습을 보여요.
이러면 결합도가 높아져서 나눈 의미가 퇴색되고, 가독성이 더 낮아질수도 있어요!
사실 지금 rows안에서 1과 -1로 이미 line을 다루고 있기에, 둘을 나누었지만 나눈게 아니게 되버렸다고 생각이 들어요.
강의에서도 row에서 이를 하나로써 보고 있는 점을 확인하실수 있을 거에요!
| return false; | ||
| } | ||
| // 현재 위치의 왼쪽에 가로선이 있는지를 확인 | ||
| if (col > 0 && ladder.isLineAtLeft(row, col)) { |
There was a problem hiding this comment.
코드상에서 col이나 row, 즉 위치가 0이상인지 확인하는 코드들이 반복되고 있습니다.
그러다보니 이 경우에서는 isLineAtLeft안에도 0보다 큰지 확인하고 있음에도, 또다시 확인하는 경우도 생겼어요.
그래서 row나 col 원시타입으로 쓰기보다는 클래스로 wrap해서 사용하는 것을 추천 드려요.
그렇게 된다면 매번 확인할 필요없이, 이에 대한 validtation 책임을 wrap한 객체에 맡기고
다른 객체에서는 이에 대해 더 생각할 것 없어질 것 같습니다.
| private static boolean validateCurrentRow(int currentRow, int[][] rows) { | ||
| if(currentRow < rows.length){ | ||
| return true; | ||
| } | ||
| else{ | ||
| throw new IllegalArgumentException(ExceptionMessage.INVALID_LINE_POSITION.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
else를 안쓰고 throw를 바로 써도 좋아요.
무조건 이렇게 써야하는거는 아니지만, 이런식으로 쓰면 편한 경우가 자주 있어서 알아두시면 좋아요. 👍
| private static boolean validateCurrentRow(int currentRow, int[][] rows) { | |
| if(currentRow < rows.length){ | |
| return true; | |
| } | |
| else{ | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_LINE_POSITION.getMessage()); | |
| } | |
| } | |
| private static boolean validateCurrentRow(int currentRow, int[][] rows) { | |
| if(currentRow < rows.length){ | |
| return true; | |
| } | |
| throw new IllegalArgumentException(ExceptionMessage.INVALID_LINE_POSITION.getMessage()); | |
| } |
| public interface LadderCreator { | ||
| Ladder createLine(int rows, int columns); | ||
| } |
There was a problem hiding this comment.
이렇게 인터페이스로 따로 나누자고 하는 것은 사용자가 직접 선을 그리는 경우, 랜덤으로 생성되는 경우 각각을 creator로 생각하고 이에 대해 조합을 이용해서 코드를 짜보자 라는것이 사실 목적이에요.
각각의 경우가 뭔지에 상관없이 LadderCreator를 받아와서 사용하는 DI를 구현하고자하는거에요.
각 경우에 대한 구현은 이를 상속받는 Random과 Custom(임시적인 클래스명)에서 구현하면 되는거죠.
만약 다른 경우가 추가된다면 다시 LadderCreator를 상속받는 클래스를 만들어서 구현하면되는 것이죠.
헷갈리시다면, SOLID의 DIP법칙을 생각하시면 됩니다!
하지만, 이 경우는 Random 경우에만 ladderCreator를 상속받는 creator가 있고 바로 ladder를 만들고 바로 반환해요.
ladder를 자동으로 만들어주기만 하는 클래스를 만든 것이죠.
이런 경우 사실 오히려 가독성을 해칠수도 있어요!
인터페이스를 만들고 사용하고자 할때, 이를 상속받고 만들어지는 클래스들에 대해 생각해보시면 좋을 것 같아요
| } | ||
|
|
||
| public boolean isLine(int row, int col) { | ||
| return rows[row][col] == 1 || rows[row][col] == -1; |
|
피드백 감사합니다! |
2 participants
일단은 마지막 열에는 Line(사다리)가 생기지 않도록 저만의 정책을 정하고 구현을 해보았습니다.
그리고 test 코드를 이런 식으로 짜도 되는 지 잘 모르겠습니다.
부족한 점 피드백 해주시면 달게 받겠습니다!