Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1단계 미션 제출합니다. #6

Open
wants to merge 5 commits into
base: nashs789
Choose a base branch
from

Conversation

nashs789
Copy link
Collaborator

@nashs789 nashs789 commented May 27, 2024

작성한지 1 ~ 2 주 되버렸네요...

눈치 보면서 진행하려다고 늦춰져서 나머지 스텝은 빠르게 진행 하겠습니다.
늦은 시간이라 내일 태그 하도록 하겠습니다.

@nashs789 nashs789 self-assigned this May 27, 2024
Copy link
Collaborator

@shoeone96 shoeone96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

  1. 저희가 객체지향 생활체조 원칙을 규칙으로 넣은만큼 들여쓰기가 들어간 부분은 분리하면 좋을 것 같습니다!(각 역할을 더 잘게 쪼개서 클래스나 메서드로 분리하면 어떨까요?)
  2. 각 클래스에 테스트 케이스가 추가되면 더 좋을 것 같습니다
    (랜덤값 -> 도출한 값이 정말 서로 다른 세 자리수인지 확인하는 테스트 등)
  3. stream을 다양한 방법을 활용해서 보여주시는 부분이 인상적이었습니다!

Comment on lines +23 to +32

if(Command.isEndApplication(userInput)) {
System.out.println("어플리케이션이 종료되었습니다.");
break;
}

if(Command.isValidInput(userInput)) {
System.out.println("1 또는 9만 입력 가능합니다.");
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation을 먼저해주는 건 어떨까요?
값에 대한 validation -> 결과물로 게임 지속 or 게임 종료 판단도 괜찮은 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네, 맞습니다.
유효성 검사가 필수로 진행되어야 하는데, 하지 못했고, step2 에 유효성 검사 코드를 넣었습니다. :)

Comment on lines +49 to +59
System.out.print("숫자를 입력해주세요, : ");
User user = new User(br.readLine());

Record record = computer.checkAnswer(user.getuserNums());
record.printResult();

if(Game.END.equals(record.getResult())) {
System.out.println("3개의 숫자를 모두 맞히셨습니다.");
System.out.println("-------게임 종료-------");
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이부분 들여쓰기가 깊어지는 것 같아서 저희 규칙처럼 메서드로 빼도 좋을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 고려하지 못한 부분인데 고민좀 해보겠습니다.

감사합니다. 👍

Comment on lines +15 to +17
Character[] userNums = inp.strip().chars()
.mapToObj(e -> (char) e)
.toArray(Character[]::new);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 toCharArray() 대신 Character[]로 만드신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream API 이용할 때 래퍼 클래스로 되어있는 배열을 파라미터로 넘기기 위해서 작성 했습니다.

public boolean isDuplicated(Character[] userNums)
public boolean isZeroInit(Character[] userNums)

래퍼 클래스가 아닌 경우 Stream 생성이 불가능하고, char 타입의 경우 CharStream 이 없기 때문에 IntStream 으로 생성해야 하는데, IntStream 으로 생성하고 아스키 코드로 확인하는건 명시적인 느낌이 안들어서 해당 코드처럼 작성 했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요거는 validation 중 isAllDigits 내부에서 사용 중인 Character.isDigit(letter) 이랑 Character.getNumericValue(e) 이 매개변수로 모두 기본형을 받고 있어서 생각났던 내용입니다!
stream으로 모두 사용한 걸 의도하셨으면 괜찮을 것 같습니다!

return Arrays.stream(userNums)
                     .filter(e -> Character.getNumericValue(e) == ZERO)
                     .count() > ZERO;

요거 intellij는 anyMatch()를 추천해주긴 하네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyMatch 추천 감사합니다.. 제 툴에서는 안해주던데 버전이 낮아서 그런가봐요 ㅎㅎ

확실히 anyMatch 사용하는게 더 낫겠네요.

@nashs789
Copy link
Collaborator Author

'객체지향 생활체조' 조금 더 고려해봐야 할 것 같습니다.

사실 step2도 이미 끝낸 상태로 부족한 부분은 step2 에 이미 채워 넣었는데 step2 코드를 step1 로 밀어 넣기보다는 다음 단계로 넘어가는게 좋을것 같다고 생각해서 바로 다음 스텝으로 가겠습니다.

리뷰 해주시느라 고생하셨습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants