Skip to content

Step1 : 1단계 - 리팩터링(상품)#331

Open
wotjd4305 wants to merge 8 commits intonext-step:wotjd4305from
wotjd4305:step1
Open

Step1 : 1단계 - 리팩터링(상품)#331
wotjd4305 wants to merge 8 commits intonext-step:wotjd4305from
wotjd4305:step1

Conversation

@wotjd4305
Copy link

안녕하세요!

이전 단계가 마무리 되지 않아, 예시 코드로 진행했습니다!

테스트 tobe아래에 생성해서하는데도, 있던 테스트들이 꽤나 변경해야하더라구요, 이 경우에는 전부다 변경하면서 진행하는게 맞을까요??

확인 부탁드립니다.

@sihoony
Copy link

sihoony commented Feb 18, 2025

안녕하세요!

이전 단계가 마무리 되지 않아, 예시 코드로 진행했습니다!

테스트 tobe아래에 생성해서하는데도, 있던 테스트들이 꽤나 변경해야하더라구요, 이 경우에는 전부다 변경하면서 진행하는게 맞을까요??

확인 부탁드립니다.

변경이 필요한 부분은 변경해주시면 될 것 같아요~! 😄
이전 단계가 마무리되지 않았다면 빠르게 마무리하시고 진행하시면 좋을 것 같습니다 👍

Copy link

@sihoony sihoony left a comment

Choose a reason for hiding this comment

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

안녕하세요~!
이번 미션을 잘 수행해주셨네요~! 👍
간단한 코멘트 남겼으니 확인해주세요. 즐거운 하루 되세요~! 😄

private UUID id;

@Embedded
private ProductName name;
Copy link

Choose a reason for hiding this comment

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

VO 사용 👍

return id;
}

public void setId(final UUID id) {
Copy link

Choose a reason for hiding this comment

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

Setter를 의미있게 변경해보면 어떨까요?
단순 Setter가 필요없다면 다른 방식으로 풀어보는 것도 좋을 것 같네요~! 😄

Copy link
Author

@wotjd4305 wotjd4305 Mar 2, 2025

Choose a reason for hiding this comment

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

Product의 ID만을 세팅하는 경우는 없고, 생성하기 위해서 이름, 가격을 주입 받고 있습니다!
제거하고 생성자로 생성하는게 나을 것 같아요

Comment on lines +19 to +33
public ProductName(String name, PurgomalumClient purgomalumClient) {
validate(name, purgomalumClient);
this.name = name;
}

public void validate(String name, PurgomalumClient purgomalumClient) {
if (Objects.isNull(name) || name.trim().isEmpty()) {
throw new IllegalArgumentException();
}

if (purgomalumClient.containsProfanity(name)) {
throw new IllegalArgumentException();
}
this.name = name;
}
Copy link

Choose a reason for hiding this comment

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

ProductName의 역할이 무엇인지 알 수 있겠네요~! 👍

Comment on lines +43 to +54
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ProductName that = (ProductName) o;
return Objects.equals(name, that.name);
}

@Override
public int hashCode() {
return Objects.hashCode(name);
}
Copy link

Choose a reason for hiding this comment

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

꼭 필요한 선언이죠~! 👍

import java.util.Objects;

@Embeddable
public class Price {
Copy link

Choose a reason for hiding this comment

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

👍

Comment on lines +46 to +48
public void setPrice(BigDecimal price) {
this.price = price;
}
Copy link

Choose a reason for hiding this comment

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

VO는 immutable 해야하는데, VO을 변경하는 setPrice가 필요할지 고민해보면 좋을 것 같아요~!

Copy link
Author

Choose a reason for hiding this comment

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

네넵! 불변이라 새로 생성하는 방법으로 로직 및 테스트가 되어있습니다! 제거하겠습니다

Comment on lines +39 to +41
public void setName(String name) {
this.name = name;
}
Copy link

Choose a reason for hiding this comment

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

setName이 필요한지 고민해보면 좋을 것 같아요~!

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.

2 participants