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

Second seminar/#3 #4

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Second seminar/#3 #4

wants to merge 5 commits into from

Conversation

Chan531
Copy link
Contributor

@Chan531 Chan531 commented Oct 26, 2023

✨ Related Issue


📝 과제 구현 명세

  1. 기본과제
  • 2차 세미나를 전공 시험 때문에 불참하여서 차근차근 읽어보며 내용을 이해하고자 노력했습니다ㅠㅠ

  • 그래서 궁금한 점이 있는데 혹시 MemberService의 updateSOPT에 있는 member.updateSOPT(new SOPT(request.getGeneration(), request.getPart())); 이 부분은 Member 클래스 내에서 updateSOPT 메소드를 따로 만들어야 하는 건가요...??

  • 멤버 생성
    image

  • 멤버 생성 확인
    image

  • 멤버 프로필 수정
    image

  • 멤버 프로필 수정 확인
    image

  1. 심화과제
  • record로 만들고 테스트 코드를 작성해서 제대로 동작하는 지 검증해보고자 했습니다. 근데 이를 테스트하는 과정이 매끄럽지 못했던 거 같았습니다.. Transaction에 대한 이해도가 부족하여 update를 확인하는 테스트 코드에서도 데이터를 새로 생성해주는 좋지 않은 코드를 작성했는데 이를 어떻게 수정하면 좋을 지 리뷰해주시면 감사하겠습니다!!

  • 사용자 수정 API에서 만약 사용자가 전체를 수정하지 않고 일부분만 수정한다면 어떻게 코드를 작성해야할지 모르겠습니다.. null이라면 수정하지않는다! 라고 적어주기엔 코드가 너무 난잡해질 거 같아서... 다른 분들의 조언이 궁금합니다!

  • 멤버 수정
    image

  • 멤버 수정 확인
    image

  • 삭제 API가 필요한 지 모르겠습니다! 그냥 아이디만 받고 지우면 되는 거 아닌가요?!?

  • memberJpaRepository에서 주석 친 부분이 왜 에러가 나는 지 모르겠습니다... id 부분이랑 똑같이 따라썼는데 어째서...

  1. 키워드과제

🐥 이런 점이 새로웠어요 / 어려웠어요

  • 세미나에 참석하지 못하여 전반적인 내용을 혼자 파악해야하는 부분이 좀 어려웠습니다ㅠㅠ
  • 다음엔 코드리뷰조 분들에게 미리미리 여쭤봐야겠다는 생각이 들었습니다!

Copy link

@sss4920 sss4920 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다..! yaml은 조심.. 또 조심..! 그래도 테스트 코드 작성하는 것도 그렇고 고민을
많이 하신거 같아서 저도 보면서 많이 배우네용!!
시험기간이었는데 심화과제까지 도전하시는 모습 멋집니다!!🔥🔥🔥🔥

show_sql: true
Copy link

Choose a reason for hiding this comment

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

헉 yml..! 이거 나중에 숨기지 않으면 재앙이 되어 돌아오니까 조심..ㅋㅋ; .gitignore에 숨기는 거 잊으면 안돼요..!


@SpringBootTest
class MemberServiceTest {

Copy link

Choose a reason for hiding this comment

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

테스트 코드..! 저도 요즘 작성하는 거 연습하고 있는데..! 좋은 습관이네요..!☺️

return findByNickname(nickname).orElseThrow(
() -> new EntityNotFoundException("존재하지 않는 회원입니다."));
}
*/
}
Copy link

Choose a reason for hiding this comment

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

이 메서드가 문제인 걸까요?! 흠흠 저도 고민을 해봤는데 이렇게 하니까 에러는 해결되긴 하는거 같습니다만..ㅠ

public interface MemberJpaRepository extends JpaRepository<Member, Long> {
	Optional<Member> findByNickname(String nickname);
	default Member findByIdOrThrow(Long id) {
		return findById(id).orElseThrow(
			() -> new NotFoundException(Error.USER_NOT_FOUND, Error.USER_NOT_FOUND.getMessage()));
	}
	default Member findByNicknameOrThrow(String nickname) {
		return findByNickname(nickname).orElseThrow(
			() -> new NotFoundException(Error.USER_NOT_FOUND, Error.USER_NOT_FOUND.getMessage()));
	}
}

.orElseThrow를 하려면 그 객체가 Optional이어야하는데 findByNickname을 그래서 Optional을 반환하도록 요렇게 만들어주니까 에러가 통과하는거 보면 findById는 위 클래스가 상속받은 CRUDRepository에서 가져와 Optional을 반환해서 그런거같다는 생각이 들긴하는데 저도 궁금하네용..ㅎㅎ;
image

Copy link
Member

@ChaeAg ChaeAg left a comment

Choose a reason for hiding this comment

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

세미나를 참여하지 못해서 과제를 혼자 하는 데에 어려움이 있었을 텐데도 너무 잘 해주셨네요!
코드를 짜면서 코드에 대한 생각과 고민을 많이 하시는 것 같아 본 받고 싶은 점입니다..😊👍
이번 주차 과제도 너무 수고 많으셨습니다!

@Transactional
public void updateMember(Long memberId, MemberUpdateRequest request) {
Member member = memberJpaRepository.findByIdOrThrow(memberId);
member.updateMember(request.name(), request.nickname(), request.age(), request.sopt());
Copy link
Member

Choose a reason for hiding this comment

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

이런 방식으로 member의 정보를 수정하기 위해서는 Member 엔티티 클래스 안에 updateMember 메서드를 선언 해주어야 하는게 맞습니다!

// Member.java
public void updateMember(String name, String nickname, int age, SOPT sopt) {
    this.name = name;
    this.nickname = nickname;
    this.age = age;
    this.sopt = sopt;
}

"남궁찬",
"갓슈",
24,
new SOPT(33, Part.SERVER)));
Copy link
Member

Choose a reason for hiding this comment

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

MemberProfileUpdateRequest클래스와 updateSOPT메서드만 검증을 하는 테스트라면 바로 Member 엔티티 클래스의 생성자를 사용해서 멤버를 생성해도 괜찮을 것 같아요!!

Member member = new Member(
                    "남궁찬",
                    "갓슈",
                    24,
                    new SOPT(33, Part.SERVER));

Copy link

Choose a reason for hiding this comment

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

오 좋은 리뷰인거 같아요..! 굳굳☺️

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.

[Feat] 2차 세미나 과제
3 participants