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

java racing car 김태근 #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

geunskoo
Copy link

@geunskoo geunskoo commented Nov 20, 2022

정신없이 짜다보니, 기능별 commit을 이번에도 실천하지 못한 것이 아쉽습니다.
저번 스터디에서 언급 했던 스트림, 람다식 표현을 최대한 반영해보려고 노력해보았습니다..!

feat:자동차 경주게임 구현이 2까지 있는 이유는
처음에 실수로 변경파일을 stage에 안올리고 commit해서 그렇습니다 🥲

@geunskoo geunskoo assigned geunskoo and unassigned geunskoo Nov 20, 2022
@geunskoo geunskoo changed the title feat: 자동차 경주게임 구현 java racing car 김태근 Nov 20, 2022
@geunskoo geunskoo self-assigned this Nov 20, 2022
Comment on lines +32 to +34
public void printPosition() {
System.out.println(this.carName + " : " + "-".repeat(carPosition));
}
Copy link

Choose a reason for hiding this comment

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

@Override
public String toString() {
  return this.carName + " : " + "-".repeat(this.carPosition);
}

toString() 메서드를 사용해보는건 어떨까요?
위 메서드를 override하면 Car 객체를 출력했을 때 위 결과가 return 됩니다!

Copy link
Author

Choose a reason for hiding this comment

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

와 정말 좋은 아이디어인 것 같습니다. 반영해보겠습니다 ㅎㅎ

Copy link

Choose a reason for hiding this comment

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

@geunskoo 그런데 구현하신 코드 보니까 지금 방법이 더 직관적인 것 같아요..!
무시하면 될 것 같아요 ㅎㅎ..

Comment on lines 22 to 27
int winnerPosition = -1;
for (Car car : carList) {
if (car.getCarPosition() >= winnerPosition) {
winnerPosition = car.getCarPosition();
}
}
Copy link

Choose a reason for hiding this comment

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

int winnerPosition = carList.stream()
        .map(Car::getCarPosition)
        .reduce(Integer::max)
        .orElse(-1);

이렇게도 가능합니다!

Copy link
Author

Choose a reason for hiding this comment

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

메서드 참조로 정말 간단하게 구현이되네요. 이 부분에 대해서 조금 더 공부해봐야겠어요.
참고 코드 감사합니다.

혹시 !! .orElse(-1)은 carList에 아무런 값이 존재 하지않아
비어있는 stream이 생성될때를 예외처리하는 기능일까요 ?

Copy link

@hyuunnn hyuunnn Nov 21, 2022

Choose a reason for hiding this comment

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

말씀하신대로 reduce는 초기값 설정을 안하면 Optional로 감싸진 형태로 반환을 합니다. orElse는 Optional에 구현된 메서드이구요 ㅎㅎ

https://codechacha.com/ko/java8-stream-reduction/
https://stackabuse.com/java-8-streams-guide-to-reduce/
참고하시면 될 것 같아요!

Copy link

@hyuunnn hyuunnn Nov 21, 2022

Choose a reason for hiding this comment

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

Optional은 비용이 커서 reduce(-1, Integer::max) 이렇게 해도 되겠네요 ㅎㅎ

https://mangkyu.tistory.com/203 참고하시면 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

친절한 설명 감사합니다 ㅎㅎㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

추후에.. 모던 자바 인 액션 스터디를 통해 관련 부분 같이 공부하면 너무 좋을 것 같네요 ㅎㅎㅎ🤪

Comment on lines 29 to 35
List<String> winners = new ArrayList<>();
for (Car car : carList) {
if (car.getCarPosition() == winnerPosition) {
winners.add(car.getCarName());
}
}
System.out.print(winners.stream().collect(Collectors.joining(", ")));
Copy link

Choose a reason for hiding this comment

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

System.out.print(carList.stream()
            .filter(car -> car.getCarPosition() == winnerPosition)
            .map(car -> car.getCarName())
            .collect(Collectors.joining(", ")));

이렇게도 가능합니다 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

오우 깔끔합니다.. 감사합니다



return Arrays.stream(inputCarName.split(","))
.map(name -> new Car(name))
Copy link

Choose a reason for hiding this comment

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

.map(Car::new) 이렇게도 사용할 수 있습니다 ㅎㅎ

Copy link
Contributor

@shkisme shkisme left a comment

Choose a reason for hiding this comment

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

제가 코로나때문에 ... 깃허브 확인이 늦었습니다 🙇‍♀️
이현 님이 코드리뷰 너무 잘 해주셨네요! 👍 제 리뷰도 확인 부탁드려요!

Random random = new Random();

if (random.nextInt(10) >= 4){
return 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

이것도 0,1 이 어떤 의미를 나타내는지 한눈에 파악하기 어려운 것 같습니다. Enum으로 표현하면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

Enum 사용이 아직 낯선것 같습니다 ...ㅎㅎ 사소한것에도 적용 하는 연습을 해야겠어요
감사합니다!

import java.util.stream.Stream;

public class GameValidator {
public static boolean carNameValid(String inputCarName){
Copy link
Contributor

Choose a reason for hiding this comment

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

boolean 반환 메서드 네이밍은 is~로 하시면 좋아요😁

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다!! 수정하겠습니다!!

try {
Integer.parseInt(playTimes);
} catch (NumberFormatException e) {
System.out.println("유효하지 않은 입력입니다. 재입력 입력하세요.");
Copy link
Contributor

Choose a reason for hiding this comment

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

지금은 상관 없지만, 추후 다른 프로젝트에서 개발을 한다고 생각하면, 최대한 상수화를 하는 것이 좋습니다! 변동 가능성이 있기 때문입니다. static final 키워드로 상수화를 하고, 이를 사용하면 좋을 것 같네요!

public static final String RETRY_MESSAGE = "유효하지 않은 입력입니다. 재입력 입력하세요.";

만들어두신 GameUtil 클래스에서 상수들을 관리하면 좋을 것 같습니다 ㅎㅎ👍

Copy link
Author

Choose a reason for hiding this comment

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

오우... 문장도 상수로써 관리하면 더 좋군요!! 신기합니다.
피드백 감사합니다

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.

3 participants