-
Notifications
You must be signed in to change notification settings - Fork 6
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
refactor(ticketItem) : 티켓상품 생성 리펙토링 #218
Conversation
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
@@ -17,7 +17,7 @@ | |||
@SecurityRequirement(name = "access-token") | |||
@Tag(name = "티켓 상품 관련 컨트롤러") | |||
@RestController | |||
@RequestMapping("/v1/ticketItems") | |||
@RequestMapping("/v1/{eventId}/ticketItems") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event/{eventId}/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
근데이제 관리자 ( 호스트, 어드민 )
호스트 컨트롤러 밑에 넣는게 좋을려나요?
이런식으로.. 너무 길어지남?
@sanbonai06 @cofls6581 @gengminy 다들 의견 어떠신가유
api/v1/host/{hostid}/event/{eventId}/ticketItems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
host는 티켓을 만드는 주체이지 host와 관련된 레코드에 대해 다루는게 아니기 때문에 지금처럼 이벤트 하위에 만드는게 좋을 거 같습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 관리자 에서 필요한 경우
- POST : api/v1/event/{eventId}/ticketItems/{ticketId}/option
- GET : api/v1/event/{eventId}/orders/
- GET : api/v1/event/{eventId}/issuedTickets/
return TicketItem.builder() | ||
.type(createTicketItemRequest.getType()) | ||
.name(createTicketItemRequest.getName()) | ||
.description(createTicketItemRequest.getDescription()) | ||
.price(Money.wons(createTicketItemRequest.getPrice())) | ||
.quantity(createTicketItemRequest.getSupplyCount()) | ||
.supplyCount(createTicketItemRequest.getSupplyCount()) | ||
.purchaseLimit(createTicketItemRequest.getPurchaseLimit()) | ||
.isSellable(true) | ||
.event(event) | ||
.build(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정적 팩터리 메서드 사용하면 더 좋을 것 같긴합니당!
public void checkTicketPrice() { | ||
if (!Money.ZERO.equals(this.price)) { | ||
throw InvalidTicketPriceException.EXCEPTION; | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좀 더 나은 방법은
객체 참조로
item -> event -> host 를 가져온뒤에
host 가 파트너인지여부를 TicketItem 객체 내에서 가져오는것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가적으로
정적 팩터리 메서드
TicketItem.builder()
를 of from 등으로 만들어 사용하면 생성시에 밸리데이션을 넣을 수 있다는 장점이 있을 것 같아요
public static TicketItem from(~~~) { //정적팩터리 메서드
var ticketItem = TicketItem.builder()~; // 기존 빌더 사용
ticketItem.validCanCreate(); // 생성가능한지 검증
return ticketItem;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리팩토링 수고하셨습니당 LGTM ⭐
개요
작업사항
변경로직