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

[6주차 과제] 박수지 과제 제출합니다. #7

Open
wants to merge 2 commits into
base: suji
Choose a base branch
from

Conversation

io-uty
Copy link

@io-uty io-uty commented Aug 18, 2024

No description provided.

@genius00hwan
Copy link

타켓 브랜치 수정하고, 다시 pr 날려주세여

@genius00hwan genius00hwan changed the base branch from main to suji August 20, 2024 12:10
@genius00hwan
Copy link

제가 했습니다,,
ㅡㅡ

Copy link

@genius00hwan genius00hwan left a comment

Choose a reason for hiding this comment

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

LGTM
일단 approve 드릴테니 리뷰 반영해서 다음 과제 진행해보세요

@@ -26,9 +29,41 @@ public String createForm() {
public String create(MemberForm form) {
Member member = new Member();
member.setName(form.getName());

Choose a reason for hiding this comment

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

setter는 최대한 지양해주세요 생성자만으로 충분해 보입니다!!

.findAny();
}
@Override
public Optional<Member> findByPwd(String pwd) {

Choose a reason for hiding this comment

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

비밀번호로 사용자를 찾는 일이 필요할까요?
비밀번호는 같은 비밀번호를 사용할 수 있을텐데 이때 findAny()를 사용해도 될까요?

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