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

refactor(api) : 발급 티켓 API 관련 리팩토링 #102

Merged
merged 6 commits into from
Jan 13, 2023

Conversation

sanbonai06
Copy link
Member

개요

작업사항

  • 도메인 VO들을 각 도메인 내 메서드를 통해 생성하는 로직으로 변경했습니다.
  • 자잘한 코드들 수정했습니다.
  • 티켓 생성 로직을 전면 수정했습니다.
    • 기존 방식 : RequestDTO처럼 새로운 매핑 객체를 받아서 생성
    • 바뀐 방식 : order + orderLineItem 객체 리스트 (DTO)로 받아옴 => orderLineItem의 quantity를 통해 티켓 생성 (해당 티켓은 옵션에 대한 답변이 동일하므로 for문으로 생성 == 맘에안듦. 더 생각해보자.)
    • 찬진이를 편하게 해주려고 바꾼 로직인데 상당히 마음에 안듭니다. 일단 저질러놓고 리팩토링하자 라는 생각으로 작업했습니다.
  • 발급 티켓 리스트 API에 전화번호로 검색을 추가했습니다. => QueryDsl 시급 (주말중으로 작업하겠습니다.)
  • 질문 : 티켓 번호나 티켓 이름으로 검색같은 기능은 필요 없을까요??

변경로직

  • 내용을 적어주세요.

@sanbonai06 sanbonai06 added For: API [이슈 대상] 외부 API Type: Feature [이슈 목적] 새로운 기능 추가 Type: Refactor [이슈 목적] 프로덕션 코드 리팩토링 labels Jan 12, 2023
@sanbonai06 sanbonai06 self-assigned this Jan 12, 2023
Copy link
Member

@cofls6581 cofls6581 left a comment

Choose a reason for hiding this comment

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

리팩토링 고생하셨습니다!!

Copy link
Member

@ImNM ImNM left a comment

Choose a reason for hiding this comment

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

굿굿

Comment on lines 158 to 167
return IssuedTicketInfoVo.builder()
.issuedTicketId(issuedTicket.getId())
.issuedTicketNo(issuedTicket.getIssuedTicketNo())
.uuid(issuedTicket.getUuid())
.ticketName(issuedTicket.getTicketItem().getName())
.ticketPrice(issuedTicket.getPrice())
.createdAt(issuedTicket.getCreatedAt())
.issuedTicketStatus(issuedTicket.getIssuedTicketStatus())
.optionPrice(issuedTicket.sumOptionPrice())
.build();
Copy link
Member

Choose a reason for hiding this comment

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

IssuedTicketInfoVo 안에 from 정적 스테직 메서드 만들어서
집어넣으면 더깔금해질것같아유!

Comment on lines +169 to +172

public static CreateIssuedTicketResponse orderLineItemToIssuedTickets(
CreateIssuedTicketDTO dto) {
long quantity = dto.getOrderLineItem().getQuantity();
Copy link
Member

Choose a reason for hiding this comment

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

IssuedTicket 도메인에 뷰를 위한 응답값을 의존하는게 쬐금 맘에 걸리긴하네유

@@ -72,4 +73,14 @@ public void login() {
throw ForbiddenUserException.EXCEPTION;
}
}

public UserInfoVo toUserInfoVo(User user) {
return UserInfoVo.builder()
Copy link
Member

Choose a reason for hiding this comment

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

UserInfoVo
안에
publice static UserInfoVo from(User user){
return 빌더넣기
}
요래 바꿀수 있을듯!


private final Order order;

private final OrderLineItem orderLineItem;
Copy link
Member

Choose a reason for hiding this comment

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

약간 그림이 주문 도메인 하나만받아서
제가 카드 -> 주문 변환하는것처럼
그런걸생각했긴 했어유!
지금방식도 좋습다!

Copy link
Member

@gengminy gengminy left a comment

Choose a reason for hiding this comment

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

구현 수고하셨습니다

.createdAt(issuedTicket.getCreatedAt())
.issuedTicketStatus(issuedTicket.getIssuedTicketStatus())
.optionPrice(issuedTicket.sumOptionPrice())
.build();
Copy link
Member

Choose a reason for hiding this comment

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

저같은 경우에도 찬진님처럼 매개변수로 도메인 객체 받는
static 메서드 of 구현해서 생성하는 방식을 선호하는데
한번 고려해 보세용

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 144 Code Smells

7.1% 7.1% Coverage
0.0% 0.0% Duplication

@sanbonai06 sanbonai06 merged commit 0dc40f9 into dev Jan 13, 2023
@sanbonai06 sanbonai06 deleted the refactor/98-refactor-issued-ticket branch January 13, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For: API [이슈 대상] 외부 API Type: Feature [이슈 목적] 새로운 기능 추가 Type: Refactor [이슈 목적] 프로덕션 코드 리팩토링
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🔨[refactor] UseCase에 과중된 로직들 mapper로 리팩 및 깔끔하게 코드 수정
4 participants