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

5단계 - 자동차 경주(리팩토링) #6102

Merged
merged 6 commits into from
Mar 24, 2025
Merged

Conversation

ParkSeryu
Copy link

@ParkSeryu ParkSeryu commented Mar 23, 2025

안녕하세요!
벌써 자동차 경주 마지막 단계네요 🥲

이번 미션은

  • 말씀해주신 strategy 패턴 적용
  • 원시 값 포장,
  • Service 객체에 대한 책임 조정을
    주안점으로 두고 리팩토링 진행했습니다!

strategy패턴은에 기존 사용하던 NumberGenerator Interface에서 크게 다르지 않고,
조금 더 범용적으로 사용할 수 있는 관점에다가 플러스
구현체에서 이동 여부를 판단하는 역할 분배를 하는 정도의 느낌을 받았습니다 (잘못 이해했다면 피드백 부탁드립니다!)

남은 부분은 코드리뷰 피드백 주신 하신 부분과
피드백 강의 들으면서 인사이트 얻은 부분들을 적용시켰습니다.

극단적으로 리팩토링 하다 보니
원래 안쓰던 방식이라 이게 괜찮은가..싶기도 했고
변경 라인이 많아서 보시기 괜찮으실까 염려스럽기도 하네요 😂

이번 Step도 잘 부탁 드립니다.👍

Copy link
Member

@nooose nooose left a comment

Choose a reason for hiding this comment

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

준형님 안녕하세요
이전 단계부터 MVC 패턴을 유지하며, 값 객체와 전략 패턴 까지 적극적으로 활용해 주셨네요
마지막 미션 깔끔하게 마무리된 것 같습니다
큰 피드백은 드릴 부분은 없어서 이대로 마무리해도 좋을 것 같아요
수고하셨습니다!!

Comment on lines +38 to +43
public List<Car> addWinner(Position maxPosition, List<Car> winners) {
if (this.position.equals(maxPosition)) {
winners.add(this);
}
return winners;
}
Copy link
Member

Choose a reason for hiding this comment

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

addWiner를 Car 책임으로 보기에는 약간 어색해 보입니다.
이전 방식처럼 Cars에서 maxPosition인 자동차를 찾아 처리해도 되지 않을까요?

// 예시
if (car.isSampePosition(maxPosition)) {
    winners.add(car);
}

Comment on lines +19 to +24
public Car move(MovingStrategy strategy) {
if (strategy.movable()) {
return new Car(this.name, this.position.increasePosition());
}
return this;
}
Copy link
Member

Choose a reason for hiding this comment

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

전략 패턴 활용 너무 좋네요 👍 👍

Comment on lines +4 to +6
private static final int INIT_POSITION = 1;
private final int value;

Copy link
Member

Choose a reason for hiding this comment

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

상수와 멤버변수간에는 개행으로 구분짓는것이 컨벤션입니다 😄

Comment on lines 9 to +15
public class RaceService {
private final NumberGenerator numberGenerator;
private static final int RAND = 10;

public RaceService(NumberGenerator numberGenerator) {
this.numberGenerator = numberGenerator;
private final MovingStrategy movingStrategy;
private Cars cars;
private final int moveCount;
private int round = 0;
Copy link
Member

Choose a reason for hiding this comment

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

서비스인데 cars나 round 같은 상태 때문에 재사용하지 못할 서비스같네요
이런 부분들도 미션 진행하면서 고려해 보셨으면 좋겠습니다 🤔

  • 서비스는 어떤 기준으로 생성하면 좋을까?
  • 도메인 로직으로는 해결하지 못할까?

@nooose nooose merged commit 1a7b85e into next-step:parkseryu Mar 24, 2025
@ParkSeryu ParkSeryu deleted the step5 branch March 24, 2025 12:37
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