-
Notifications
You must be signed in to change notification settings - Fork 97
[그리디] 이채현 JPA4-5단계 미션 제출합니다 #266
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
base: chaehyunl
Are you sure you want to change the base?
Changes from all commits
d0eac68
53bba1c
24ee8ee
54aee6d
0365054
143eb7d
ecc8025
80053ba
ab18aef
66728ba
74f36bd
f0ce4c0
44ad820
c5ae254
05a06d3
7e0c71b
95f915d
165b552
40aff17
578d9a3
af5ff58
b0692c5
d87a302
4836e92
d808ed6
4c1ffe7
f77ce23
0de02ba
2e26756
4730128
0eae6c3
653e960
8bb5385
488f861
aa2b5fa
8895666
4184b79
3bd4251
ec8004e
8957601
20a948c
f948434
93465ad
2fba555
49b6d7f
13b8925
715960c
a272863
cee6ff6
9fda054
767742f
a3eebbe
262a651
c5cc7a3
6fff1f3
4518389
b9cc04b
31d685e
9c9ae75
e3f65ef
35ebc38
0f3d928
77abb17
cae9a52
dd40b5a
07c4f55
1ff8645
1373c71
3db99b7
80797fa
cd1a819
74c0595
313e5ee
fd3e903
12e9014
7bb3fe5
72b8311
63b2eb1
13f3d39
5405303
6c6a2e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package roomescape.member; | ||
|
|
||
| import jakarta.persistence.EntityManager; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
| @Repository | ||
| public class MemberRepository { | ||
| private final EntityManager entityManager; | ||
|
|
||
| public MemberRepository(EntityManager entityManager) { | ||
| this.entityManager = entityManager; | ||
| } | ||
|
|
||
| public Optional<Member> findById(Long id) { | ||
| return Optional.ofNullable(entityManager.find(Member.class, id)); | ||
| } | ||
|
|
||
| public void save(Member member) { | ||
| entityManager.persist(member); | ||
| } | ||
|
|
||
| public List<Member> findAll() { | ||
| return entityManager.createQuery( | ||
| "SELECT m FROM Member m", | ||
| Member.class | ||
| ).getResultList(); | ||
| } | ||
|
|
||
| public Optional<Member> findByEmailAndPassword(String email, String password) { | ||
| List<Member> members = entityManager.createQuery( | ||
| "SELECT m FROM Member m WHERE m.email= :email AND m.password =:password", | ||
| Member.class | ||
| ).setParameter("email", email) | ||
| .setParameter("password", password) | ||
| .getResultList(); | ||
|
|
||
| return members.stream().findFirst(); | ||
| } | ||
|
|
||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,21 @@ | |
|
|
||
| @Service | ||
| public class MemberService { | ||
| private MemberDao memberDao; | ||
| private final MemberRepository memberRepository; | ||
|
|
||
| public MemberService(MemberDao memberDao) { | ||
| this.memberDao = memberDao; | ||
| public MemberService(MemberRepository memberDao) { | ||
| this.memberRepository = memberDao; | ||
| } | ||
|
|
||
| public MemberResponse createMember(MemberRequest memberRequest) { | ||
| Member member = memberDao.save(new Member(memberRequest.getName(), memberRequest.getEmail(), memberRequest.getPassword(), "USER")); | ||
| Member member = | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. POST /members를 호출해 보셨나요? 회원가입이 이루어지지 않습니다. 이유는 어떤 것일까요?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dao를 제가 여전히 호출하고 있어서 그런 것 같습니다..! . 다음 리뷰할때 그부분 참고해서 변경하도록 하겠습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 한 번 호출해서 예외를 확인해보시면 좋을 것 같아요~!
이 부분은 다음 리뷰어분께 확인을 요청드리겠습니다! |
||
| new Member( | ||
| memberRequest.getName(), | ||
| memberRequest.getEmail(), | ||
| memberRequest.getPassword(), | ||
| "USER"); | ||
|
|
||
| memberRepository.save(member); | ||
| return new MemberResponse(member.getId(), member.getName(), member.getEmail()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
orElseThrow()에 예외 메시지가 없네요수정하게 된다면 예외 메세지가 없는 모든 코드에서 수정이 이루어지길 바래요~!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 코드를 짤때는 member를 optional로 받도록 되어있습니다. 즉 null값이 존재할 수 있습니다. 그런데 이걸 orElseThrow가 없게된다면, null값일때 대처할 수 없게됩니다.
만약 제가 optional로 받지 않았더라면 orElseThrow를 없애는게 맞다고 생각합니다.
하지만, 제가 생각했을때, member를 전달하는 과정에서 오류가 생기면 그저 Member member를 썼을때는 대처할 수 없게된다고 생각되어서 optional를 사용해서 오류검증 orElseThrow가 있는 방식이 조금더 적합된다고 생각됩니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional로 처리해주신 부분 좋은 설계라고 생각해요!
findBy~라는 네이밍 자체가 "반환값이 있을 수도, 없을 수도 있다"는 걸 말해주니까요~제가 말씀 드리고 싶은 부부능ㄴ
orElseThrow()괄호 안이 비어 있다는 것이었어요!아래 코드와 같이 커스텀 예외나 메시지를 넣어줄 수도 있지 않을까 싶더라구요
지금처럼 인자 없이
orElseThrow()만 쓰면 후자가 주는 이점들을 놓치게 되지 않을까 싶은데채현님은 어떻게 생각하시나요?
그리고 그 생각을 답변에 최대한 상세하게 작성해주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순히 optional을 사용했을때, .orElseThrow()를 쓰면 오류가 나지 않는다는 면에서 orElseThrow를 사용하였습니다.
만약, orElseThrow를 이용해서 새로운 커스텀 에러 메세지를 넣는다면, 오류가 나는 지점을 메세지를 받음으로써 더 정확하고 쉽게 파악할 수 있다고 생각합니다!
커스텀 예외를 넣는 것이 더 좋다고 생각되네요!!