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 #5836

Open
wants to merge 20 commits into
base: sweet-info
Choose a base branch
from
Open

Step5 #5836

wants to merge 20 commits into from

Conversation

sweet-info
Copy link

피드백 감사합니다.
추가로 리뷰 요청드립니다.
감사합니다 :)

Copy link
Contributor

@seondongpyo seondongpyo left a comment

Choose a reason for hiding this comment

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

마지막 단계도 잘 진행해주셨습니다.
몇 가지 코멘트 드렸는데 확인 후 다시 리뷰 요청을 부탁 드릴게요 🔥

throw new IllegalArgumentException("Car name cannot be null or empty.");
throw new IllegalArgumentException("cannot be null or empty.");
}
if (value.length() > 5) {
Copy link
Contributor

Choose a reason for hiding this comment

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

5와 같은 숫자 리터럴은 상수로 선언하여 의미를 부여해보면 어떨까요?

Comment on lines +17 to +18
@Test
void carNameCannotBeNull() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@NullAndEmptySource를 활용하여 코드 중복을 줄여보면 어떨까요?

@@ -20,7 +20,9 @@ void carDoesNotMove() {
Car car = new Car("Car") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Car에 전략 패턴을 적용하여 자동차의 움직임을 유연하게 만들어보면 어떨까요?
https://park-algorithm.tistory.com/entry/%EC%A0%84%EB%9E%B5-%ED%8C%A8%ED%84%B4-%EC%82%AC%EC%9A%A9-%ED%9B%84%EA%B8%B0

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