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

Sohyupar spring #9

Open
wants to merge 7 commits into
base: sohyupar
Choose a base branch
from
Open

Sohyupar spring #9

wants to merge 7 commits into from

Conversation

saewoo1
Copy link
Contributor

@saewoo1 saewoo1 commented Jan 10, 2024

과제 끄..ㅌ?

Copy link
Contributor

@Ssuamje Ssuamje left a comment

Choose a reason for hiding this comment

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

LV 1 : 설명도 잘 작성해주셨고 다 맞습니다..! @value에서 default 값 지정해주는 부분은 저도 처음 알았어요bb

LV 2 : 서비스 자체에서 꼭 필요한 경우가 아니면 private 메서드로 굳이 빼지 않는 게 더 좋을 것 같습니다(아래로 내려놓으면 오히려 이해하기 어려울 수 있음, 그리고 private이 많아진다는 얘기는 서비스의 책임이 무거워졌다는 뜻) && lentHistoryRepository에서 userId를 기준으로 find(optional 반환)하는 방식으로 작성해주셨어도 좋을 것 같습니다! 이외에는 더 말씀드릴게 없어요..

LV 3 : 다 맞게 작성하셨습니다..!

온보딩 고생하셨습니다!! she is gosu

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