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

입찰 시 입찰하고자 하는 내용이 유효한지 검증하는 로직 변경 #745

Closed
wants to merge 2 commits into from

Conversation

apptie
Copy link
Collaborator

@apptie apptie commented Nov 9, 2023

📄 작업 내용 요약

Auction에서 Optional로 lastBid를 반환하도록 했고
checkInvalidBid()에서 Optional.ifPresentOrElse()를 활용해 분기 처리를 없애고 Optional의 람다식으로 처리했습니다

🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드

이슈에 왜 이런 제안을 했는지에 대한 내용을 살짝 자세히 적어놨습니다

📎 Issue 번호

@apptie apptie added refactor 기존 기능에 변경이 없는 구현 변경 시 backend 백엔드와 관련된 이슈나 PR에 사용 suggestion 코드 변경 사항이 있는 경우 팀원들에게 제안하기 위함 labels Nov 9, 2023
@apptie apptie requested review from JJ503, swonny and kwonyj1022 November 9, 2023 04:50
@apptie apptie self-assigned this Nov 9, 2023
Copy link
Member

@JJ503 JJ503 left a comment

Choose a reason for hiding this comment

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

당시에는 돈이기에 최대한 보수적으로 가져가야겠다는 생각으로 했던 것인데, 지금 보니 굳이 그럴 필요가 없어 보이네요..!
수정 제안 사항에 대해 잘 확인했으며, 동의해 approve 합니다!

해당 파트는 auction에 있긴 하지만, 제가 잤던 코드로 기억합니다.
그래서 실제 pr도 지토가 진행해 주시는 것인지 확인차 여쭤봅니다!

@apptie
Copy link
Collaborator Author

apptie commented Nov 10, 2023

@JJ503
입찰쪽에 뭔가 수정해야 할 부분이 있는 것으로 알고 있고 그것을 제이미가 진행해주시는 것 같아서
그 작업하면서 동시에 이...제안도 해주시면 될 것 같습니다

Copy link
Collaborator

@kwonyj1022 kwonyj1022 left a comment

Choose a reason for hiding this comment

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

감사합니다.
작업하시는 분 작업하실 때 BidRepository에서 findLastBidByAuctionId() 제거해 주시면 감사하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend 백엔드와 관련된 이슈나 PR에 사용 refactor 기존 기능에 변경이 없는 구현 변경 시 suggestion 코드 변경 사항이 있는 경우 팀원들에게 제안하기 위함
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants