2주차 미션 / 서버 4조 최재현 #40
Conversation
JGoo99
left a comment
JGoo99
left a comment
There was a problem hiding this comment.
요구사항 모두 만족해주셨네요! 수고많으셨습니다.
(커밋 단위는 조금 줄여도 좋을 것 같아요!)
| import ladder.Position; | ||
| import ladder.Row; | ||
|
|
||
| public interface LadderCreatorIF { |
There was a problem hiding this comment.
interface 클래스에 ~IF 를 붙이거나, 구현체에 ~Impl을 붙이는게 헝가리안 표기법이 남아있는 경우입니다.
헝가리안 표기법은 동어반복 이 되기 때문에 지양해야 하는 관습 중 하나입니다!
해당 인터페이스의 의미 자체 (LadderCreator)를 클래스명으로 사용하는 게 좋을 것 같습니다ㅎ
|
|
||
| public class LadderCreator { | ||
|
|
||
| public class LadderCreator implements LadderCreatorIF { |
There was a problem hiding this comment.
LadderCreater 를 인터페이스로 만들었다면 그것을 구현하는 구현체의 특색에 맞게 클래스명을 지어주면 유지보수에 용이할 것 같아요!
(ex. RandomLadderCreater)
| public RandomLadderCreator(LadderSize ladderSize) { | ||
| ladderCreator = new LadderCreator(ladderSize); | ||
| this.ladderSize = ladderSize; |
There was a problem hiding this comment.
LadderCreator 를 생성해서 사용할거면 매개변수로 LadderCreator 를 받아도 좋을 것 같습니다!
| } | ||
|
|
||
| private void initLadder() { | ||
| HashSet<LadderPosition> linePositionSet = new HashSet<>(); |
There was a problem hiding this comment.
참조변수인 객체는 quals, hashCode 메서드를 오버라이드해서 같은 객체인지 검증하는 기능이 필요해요.
(단순 값의 중복을 검증하는 것을 넘어 메모리 비교(해시값비교) 를 통해 객채의 중복을 검증하기 위해)
| private String getLadderString(LadderPosition ladderPosition) { | ||
| StringBuilder ladderStringBuilder = new StringBuilder(); | ||
|
|
||
| for (int i = 0; i < rows.length; i++) { | ||
| ladderStringBuilder.append(getNthRowString(i, ladderPosition)) | ||
| .append('\n'); | ||
| } | ||
| return ladderStringBuilder.toString(); | ||
| } | ||
|
|
||
| private String getNthRowString(int n, LadderPosition ladderPosition) { | ||
| String rowString = rows[n].getRowString(); | ||
| if (ladderPosition.isNthRow(n)) { | ||
| rowString = insertAsterisk(rowString, ladderPosition); | ||
| } | ||
| return rowString; | ||
| } | ||
|
|
||
| private String insertAsterisk(String rowString, LadderPosition ladderPosition) { | ||
| String[] nodeStrings = rowString.split(" "); | ||
| StringBuilder stringBuilder = new StringBuilder(); | ||
|
|
||
| for (int i = 0; i < nodeStrings.length; i++) { | ||
| stringBuilder.append(nodeStrings[i]) | ||
| .append(ladderPosition.isNthNode(i) ? "* " : " "); | ||
| } | ||
| return stringBuilder.toString(); | ||
| } |
There was a problem hiding this comment.
Runner 에서 Row, Node 의 변수 정보를 getter 로 가져와 그리기보다는,
StirngBuilder 인자를 넘겨주면서 Row 클래스 -> Node 클래스 레벨로 직접 들어가 내부 메서드를 통해 사다리와 별표를 그려주는 건 어떤가요?
조금 더 객체지향적인 코드가 될 수 있을 것 같아요!
| int maxLineCount = ladderSize.getMaxLineCount(); | ||
| int maxRowNumber = ladderSize.getNumberOfRowValue(); | ||
| int maxNodeNumber = ladderSize.getNumberOfPersonValue() - 1; | ||
|
|
||
| while (linePositionSet.size() < maxLineCount) { | ||
| int x = (int)(Math.random() * maxRowNumber); | ||
| int y = (int)(Math.random() * maxNodeNumber); | ||
| LadderPosition ladderPosition = new LadderPosition(x, y); |
There was a problem hiding this comment.
row, node, person, x, y 등 의미는 같은데 이름만 다른 변수가 코드의 직관성을 떨어뜨리는 것 같습니다!
| public int run(LadderPosition ladderPosition) { | ||
| for (int i = 0; i < rows.length; i++) { | ||
| rows[i].nextPosition(position); | ||
| printLadder("Before", ladderPosition); | ||
| rows[i].nextPosition(ladderPosition); | ||
| printLadder("After", ladderPosition); | ||
| ladderPosition.goDown(); | ||
| } | ||
| return position.getValue(); | ||
| return ladderPosition.getValueY(); | ||
| } |
There was a problem hiding this comment.
Before, After 도 enum 클래스를 만들어 사용하면 하드코딩을 피하고, 구현코드나 테스트코드 등에서 안전하게 재사용할 수 있을 것 같아요.
rows[i].nextPosition() 함수에는 y 좌표만 Position 객체로 넘겨주면 내부 메서드가 더 깔끔해질 것 같아요.
| private void validateLadderPosition(LadderPosition ladderPosition) { | ||
| validatePosition(ladderPosition.getY()); | ||
| } |
There was a problem hiding this comment.
위에 언급했듯이 인자로 Position 을 받으면 해당 메서드는 지워도 좋을 것 같습니다!
2 participants
No description provided.