Uh oh!
There was an error while loading. Please reload this page.
2단계 - 사다리(생성) - #1601
Open
jimbaemon wants to merge 12 commits into
Open
Conversation
lxxjn0
requested changes
Nov 10, 2022
lxxjn0
left a comment
There was a problem hiding this comment.
재현님, 미션 구현하시느라 수고 많으셨습니다!!
간단하게 먼저 피드백 남겨두었는데 출력 로직과 도메인 로직의 구분에 포커스를 둬서 한번 개선해보면 좋을 것 같아요!! 👍🏼
Comment on lines
+4
to
+5
| private final static int MAX_NAME_LENGTH = 5; | ||
| private final String name; |
Comment on lines
+18
to
+21
| @Override | ||
| public String toString() { | ||
| return String.format("%6s", name); | ||
| } |
There was a problem hiding this comment.
toString 메서드의 의미와 사용 용도에 대해서 한번 알아보면 좋을 것 같아요!! 현재는 출력을 위한 용도로 사용되고 있는데 toString의 올바른 용도에 맞게 사용되지 못한 것 같아요!! 아래 글을 참고해보시면 좋을 것 같습니다 👍🏼
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| public class Players { |
There was a problem hiding this comment.
toString을 올바르게 사용한다면 현재 Players 일급컬렉션은 큰 의미를 가지기가 어려울 것 같아요!!
Comment on lines
+8
to
+9
| SecureRandom secureRandom = new SecureRandom(); | ||
| return secureRandom.nextBoolean(); |
There was a problem hiding this comment.
SecureRandom 클래스는 처음 보는데 덕분에 좋은 정보를 알게 된 것 같아요!! 👍🏼
| import java.util.Objects; | ||
| public class Line { | ||
| private final Boolean hasLine; |
| } | ||
| public static Line create(final Line prevLine) { | ||
| if (prevLine == null || !prevLine.hasLine) { |
There was a problem hiding this comment.
간단한 조건문이지만 private 메서드로 뽑아서 이름을 부여한다면 코드를 읽기 더 좋을 것 같아요!!
Comment on lines
+15
to
+16
| SecureRandom secureRandom = new SecureRandom(); | ||
| return new Line(secureRandom.nextBoolean()); |
| if (index == 0) { | ||
| return null; | ||
| } | ||
| return result.get(index - PREV_INDEX); |
This file contains hidden or 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
Sign up for freeto 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.
조금 늦었습니다 :)
Integer Stream 제외하곤 함수형 stream 사용할 경우가 많이 안보이고
Integer Stream 의 경우 forEach 처리가 더 좋아보여서 ForEach 로 해봤는데 ㅜㅜ
수업 의도를 잘 못따라 가겠네요