Open
Conversation
when name is longer than 5 words, or there were same names.
PandaHun
requested changes
Apr 5, 2021
Member
PandaHun
left a comment
PandaHun
left a comment
There was a problem hiding this comment.
많은 부분에서 요구사항을 충족하지 못하고 있습니다.
요구사항 확인 후 피드백 반영 부탁드립니다.
또한 인덴트에 있어서 해당 내용을 메서드로 호출하거나 하면 인덴트 1로도 충분한 객체지향 프로그래밍이 가능합니다.
| import java.util.List; | ||
| import racingcar.Winner; | ||
|
|
||
| public class OutputView { |
Member
There was a problem hiding this comment.
정적 메서드만을 제공하는 정적 클래스입니다.
생성자를 막아줌으로써 사용의 혼동을 막아주세요.
| import racingcar.Winner; | ||
|
|
||
| public class OutputView { | ||
| public static final String bar = "-"; |
Member
There was a problem hiding this comment.
Suggested change
| public static final String bar = "-"; | |
| public static final String BAR= "-"; |
하지만 bar보다 더 좋은 네이밍이 있지 않을까요?
| public static void printWinner(Winner winner) { | ||
| System.out.println("최종 우승자"); | ||
| System.out.print(winner.getWinnersName()); | ||
| } |
Member
There was a problem hiding this comment.
해당 내용들 전부 해당 객체의 내부 구현을 매우 잘 알고 있습니다.
다른 객체에서는 해당 객체의 내부 구현을 몰라도 사용할 수 있게 구현을 개선해주세요
| } | ||
| System.out.println(); | ||
|
|
||
| if (maxReach < car.getPosition()) { |
Member
There was a problem hiding this comment.
책임이 늘어났네요?
결과를 출력해주는 객체가 아니라 게임의 결과를 판별하고 있습니다.
|
|
||
| public class Application { | ||
|
|
||
| public static void main(String[] args) { |
Member
There was a problem hiding this comment.
메서드의 10줄 이상은 메인 메서드 또한 마찬가지 입니다.
| throw new IllegalArgumentException("[ERROR] 랜덤값이 잘못되었습니다."); | ||
| } | ||
|
|
||
| if (startInclusive < 0) { |
|
|
||
| // index 0에 이름 중복 검사 결과 | ||
| // index 1에 이름 길이 검사 결과 | ||
| validationResult[0] = isNameOnly(carNames); |
Member
There was a problem hiding this comment.
이 내용에 주석이 삭제된다면 알 수 있을까요?
외국인이 코드를 유지보수 하게 된다면?
우리는 우리나라 사람이랑만 협업하지 않습니다
| for (int i = 0; i < carNames.length; i++) { | ||
| String name = carNames[i]; | ||
|
|
||
| if (isNameLong(name)) { |
| } | ||
|
|
||
| public static boolean isNameLong(String name) { | ||
| if (name.length() > 5) { |
| public static int isNameOnly(String[] carNames) { | ||
| Map<String, String> nameCount = new HashMap<String, String>(); | ||
|
|
||
| for (int i = 0; i < carNames.length; i++) { |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.