2단계 - 사다리(생성) #1562
Open
2단계 - 사다리(생성) #1562
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 10 commits
October 25, 2022 20:27
… drawing이 가지고 있도록 함
JunHoPark93
requested changes
Nov 2, 2022
Comment on lines
+10
to
+14
| public class Ladder { | ||
|
|
||
| private final List<Player> players; | ||
| private final List<Line> lines; | ||
| private final String drawing; |
There was a problem hiding this comment.
사다리 객체에 필요한 정보가 어떤것인지 정리가 필요할거같아요~
필요한 최소의 정보만을 유지하는게 어떨까요? 불필요한 필드를 가짐으로써 응집력이 낮아져 유지보수를 힘들게 할 수 있습니다.
Comment on lines
+22
to
+32
| public static Ladder from(GameSettingRequest gameSettingRequest) { | ||
| List<Player> newPlayers = gameSettingRequest.getPlayerRequests() | ||
| .stream() | ||
| .map(Player::from) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| int playerCount = gameSettingRequest.getPlayerRequests().size(); | ||
| int ladderHeight = gameSettingRequest.getLadderRequest().getHeight(); | ||
| List<Line> newLines = Stream.generate(() -> Line.of(playerCount)) | ||
| .limit(ladderHeight) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
위 내용을 개선하면 사다리 생성에 있어서 불필요한 로직들도 분리될거에요!
There was a problem hiding this comment.
GameSettingRequest는 client의 입력을 가지는 객체인데요, 필요한 정보만을 넘기는것은 어떨까요?
외부 변화에 취약한 단점이 있어보여요.
(추가로 테스트 코드 작성하실때도 불편하시지 않았나요?)
There was a problem hiding this comment.
테스트코드가 없군요!
도메인에 대한 테스트도 꼭 작성해주셔요 😊
Comment on lines
+38
to
+43
| private static String createDrawing(List<Player> players, List<Line> lines) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| addPlayerNames(sb, players); | ||
| addLines(sb, lines); | ||
| return sb.toString(); | ||
| } |
Comment on lines
+54
to
+56
| public void print() { | ||
| System.out.println(this.drawing); | ||
| } |
Comment on lines
+46
to
+49
| public String print() { | ||
| StringBuilder sb = new StringBuilder(); | ||
| sb.append(" "); | ||
| hasSteps.forEach(hasStep -> { |
| @@ -0,0 +1,26 @@ | |||
| package ladderapplication.models.requests; | |||
There was a problem hiding this comment.
requests 패키지 위치도 도메인 하위가 아닌 최소한 같은 레벨로 올려야하지 않을까요?
| import java.util.stream.IntStream; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class Line { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
2단계 - 사다리(생성)
ㄴ
Player게임에 참여하는 사용자의 이름을 필드로 가집니다.ㄴ
Line일반적인 사다리를 생각했을 때, 세로(y축) 한 줄의 정보가 아닌 가로(x축) 한 줄의 정보를 가집니다. boolean list로 x축의 특정 지점에 가로선의 존재유무를 가지고 있습니다. x축에 수직인 세로 줄은 Player 수와 일치하나 가로선은 마지막 세로줄에는 존재하지 않으므로 필드인 hasSteps의 길이는 Player 수에서 1을 뺀 것과 수가 같습니다.ㄴ
Ladder사다리입니다. 게임에 사용되는 정보를 통틀어서 가지고 있습니다.ㄴ
GameSettingRequestLadder를 만들기 위해 사용되는 정보입니다. 사용자가 입력한 값을LadderRequest,List<PlayerRequest>로 가지고 있습니다.ㄴ 2단계에서는 가로선 유무의 조건을 명시하지 않았기때문에 임의로 0~9 숫자를 뽑아 4이상인 경우 hasStep = true로 셋팅합니다.
ㄴ
Ladder가 게임의 모든 정보를 다 포함하고 있으므로 Line로 Ladder 생성 시에 함께 정의됩니다.ㄴ
Ladder가 가지고 있는 String 타입의 필드입니다.ㄴ 처음 코드를 짤 때는
print()필드에서 매번 StringBuilder를 이용해 사다리게임판을 그렸으나 게임판은 변화가 없는 값일거라 처음 Ladder를 생성할 때 필드로 그 값을 가지고 있어도 되겠다는 생각이 들었습니다.ㄴ Ladder 생성 시
createDrawing을 이용해 값을 정의하고 있으며, 일부는 Ladder 내부에 일부는DecoratingUtils로 빼내어 사용하고 있습니다. 이 부분에서 아예 DecoratingUtils에 Ladder를 넘겨 문자열을 생성하는게 좋을지 아니면 모두 Ladder에 남겨두는게 나을지 고민이 많았습니다. 결과적으론 이렇게 조금씩 나눠진 형태를 띄고있는데, 제가 생각하기에 DecoratingUtils라는 명칭에 어울리는 즉, 문자열을 꾸미고 변형하는 경우는 유틸로 빼는 것으로 결론 내린 결과물입니다.ㄴ 가로줄의 형태는
Line.print()에서 또 가져오도록 되어있는데요. (내부적으로 DecoratingUtils를 섞어서 사용 중) 이 부분도 고민이 많았습니다. Line의 hasSteps를 모두 가져와 Ladder에서 String을 뽑아낼까했지만 Line이라는 다른 객체로 정의하였으니 해당 객체에서 작업을 하여 응답하는 것이 더 객체지향적이지 않을까싶어 이렇게 처리하였습니다.drawing을 그리는 과정과 Ladder를 만드는 과정에서 고민이 많았는데요..
보완해야 할 점 말씀해주시면 다시 한 번 고민해보도록 하겠습니다.
감사합니다.