Skip to content

Comments

[BE] 비회원 null 값 넘겨주기 대신 오버로딩 사용하기#307

Merged
soyun-i merged 2 commits intodevelopfrom
be/feat/299
Apr 16, 2025
Merged

[BE] 비회원 null 값 넘겨주기 대신 오버로딩 사용하기#307
soyun-i merged 2 commits intodevelopfrom
be/feat/299

Conversation

@soyun-i
Copy link
Contributor

@soyun-i soyun-i commented Apr 15, 2025

🚀 어떤 기능을 구현했나요 ?

  • null 값을 보내주던 기존 코드에서 오버로딩을 사용하여 필요한 값들만 넘겨주게 바꿨습니다!
  • 회원 관련 메서드는 @CurrentUser User user 파라미터를 사용
  • 비회원 관련 메서드는 orderNumberphone 파라미터를 사용

🔥 어떤 문제를 마주했나요 ?

  • 그 전에는 null을 넘겨줌으로써 1. 불명확한 의도 2. 추후 유지보수 어려움 등의 문제가 있었습니다

✨ 어떻게 해결했나요 ?

  • 이제 서비스 계층으로 null을 전달할 필요가 없어짐으로써
  1. 코드의 가독성 향상
  2. null 관련 버그 발생 가능성이 줄어듦

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료 및 회고

@soyun-i soyun-i added this to the 6차 마일스톤 milestone Apr 15, 2025
@soyun-i soyun-i self-assigned this Apr 15, 2025
@Transactional
public OrderResponse createInstantOrder(User user, OrderRequest instantOrderRequest) {
//락키 생성 (상품 ID를 기반으로)
String lockKey = "order:instant:" + instantOrderRequest.orderDetailRequestList().get(0).saleProductId();
Copy link
Contributor

Choose a reason for hiding this comment

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

OrderRequest 와 많이 엮여 있습니다.
orderDetailRequestList() → get(0) → saleProductId()
한번 개선해 보는게 좋을 것 같아요

@KWAK-JINHO
Copy link
Contributor

null을 처리하는데 저도 항상 고민이 많습니다 장기적으로 유지보수성을 크게 향상시키는데 중요하다고 생각해요.

@Override
@Transactional
public OrderResponse createOrder(OrderRequest orderRequest) {
return createOrder(null, orderRequest);
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 보니 이렇게 user에 null이 있으면
guest라는 처리를 해주는 것도 좋겠네요!

Copy link
Contributor

@kiteof-park kiteof-park left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

@soyun-i soyun-i merged commit 6df89bf into develop Apr 16, 2025
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.

4 participants