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

Step5 #6076

Open
wants to merge 1 commit into
base: jihyun-s
Choose a base branch
from
Open

Step5 #6076

wants to merge 1 commit into from

Conversation

jihyun-s
Copy link

No description provided.

Copy link

@shared-moon shared-moon left a comment

Choose a reason for hiding this comment

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

지현님 안녕하세요 ~
5단계 미션 잘 진행 해 주셨네요 :)
코멘트를 좀 남겨뒀는데, 한번만 더 보고 마무리하면 좋을 것 같아요!
확인 후 리뷰 재요청 해주세요 🔥
고생 많으셨습니다 :)

this.cars = List.copyOf(cars);
}

public static Cars fromNames(List<String> names) {

Choose a reason for hiding this comment

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

👍

return new Cars(cars);
}

public Cars withAddedCar(Car car) {

Choose a reason for hiding this comment

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

테스트를 위한 메서드 생성은 지양해주세요 ~

return Collections.unmodifiableList(cars);
}

public List<String> getWinners(Cars records) {

Choose a reason for hiding this comment

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

파라미터로 다른 Cars를 받을 필요가 있을까요 ?

public List<String> getWinners(Cars records) {
int maxPosition = getMaxPosition(records);
return records.stream()
.filter(car -> car.getPosition() == maxPosition)

Choose a reason for hiding this comment

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

Car.equalsPosition 을 만들어서 getter 대신 질의를 던져보는건 어떨까요 ?

@DisplayName("우승자가 한 명이면 한 명의 이름을 반환한다.")
void getWinnerOne() {
Cars cars = new Cars(new ArrayList<>());
cars = cars.withAddedCar(new Car("pobi", 2));

Choose a reason for hiding this comment

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

getWinners 테스트를 위해 사전조건 세팅을 하려고 메서드를 추가하신 것 같아요 ~
getWinners 내에서 Car의 동작을 제어하지 못해서 이렇게 진행하신걸로 이해가 되는데 mockito를 이용해서 Car를 목킹하여 응답값을 제어해보면 어떨까요 ?
위에도 적어뒀지만, 테스트를 위한 메서드 생성은 지양하는게 좋아서요!

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