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

[Spring Core] 김수민 미션 제출합니다. #66

Open
wants to merge 16 commits into
base: boyekim
Choose a base branch
from

Conversation

boyekim
Copy link

@boyekim boyekim commented Jul 8, 2024

auth 패키지를 분리하며 RoomescapeApplication이 최상단이 아니게 되었고 따라서 Component Scan의 범위에서 벗어나게 되었습니다.
따라서 AuthConfig로 빈을 등록하고 싶은 클래스를 설정해준 후 @Import를 통해 스캔 범위 밖도 Component Scan을 해 주었습니다.
이번 미션을 통해 @ComponentScan의 범위와 대상에 대해 되새길 수 있었습니다!

또한 @Profile + CommandLineRunner를 처음 써보는데 이에 대한 사용방법도 배울 수 있는 미션이었습니다. JPA의 엔티티를 통해 sql 작성에서 벗어나 데이터를 객체의 시점으로 다룰 수 있는 것이 신기했는데 @Profile + CommandLineRunner로 인해서 데이터를 완전히 객체처럼 다루게 되는 것 같아 인상깊었습니다

다들 고생 많으셨습니다 해커톤도 화이팅

이번 미션 범위

Comment on lines 12 to 15
public class JwtUtils {

@Value("${roomescape.auth.jwt.secret}")
private String secretKey;

Choose a reason for hiding this comment

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

필드 주입 대신 생성자 주입은 어떠신가요?

생성자 주입을 하게 순환 의존성 문제를 방지할 수 있다고 합니다.

또한 final 로 선언하면 불변성을 보장할 수 있을거 같아요 : )

Copy link
Author

Choose a reason for hiding this comment

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

  1. 주입 관련해서 어떤 거 말씀하시는 걸까요??

  2. final로 선언해야겠네요! 놓친 부분이었습니다 수정하겠습니당

Comment on lines 23 to 24
public String login(@RequestBody LoginRequest loginRequest) {
Member member = memberRepository.findByEmailAndPassword(loginRequest.getEmail(), loginRequest.getPassword());

Choose a reason for hiding this comment

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

예전 미션 이지만
findByEmailAndPassword 는 null Pointer 예외가 터질수도 있어
null 체크 해보는게 좋을거 같아요 : )

findByEmailAndPassword 함수의 반환값을 Optional 사용하면 쉽게 처리할 수 있습니다 : )

Copy link
Author

Choose a reason for hiding this comment

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

저번 스터디때도 느꼈는데 Optional 사용을 더 적극적으로 해야할 것 같습니다 반영하겠습니다

Copy link

@PlusUltraCode PlusUltraCode left a comment

Choose a reason for hiding this comment

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

안녕하세요 수민씨! 리뷰어 이동호입니다.

전반적으로 패키지 분리가 잘 되어 있고, 변수명도 잘 만들어져 있어서 코드를 읽기 수월했습니다. 좋은 구조와 명확한 네이밍 덕분에 이해하기 쉬웠습니다

놓치기 쉬운 부분들에서 중점적으로 리뷰를 남겼습니다.

고생 많으셨습니다 : )

Comment on lines +22 to +27
@Override
public void run(String... args) throws Exception {
memberRepository.save(new Member("어드민", "[email protected]", "password", "ADMIN"));
memberRepository.save(new Member("브라운", "[email protected]", "password", "USER"));
}
}

Choose a reason for hiding this comment

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

데이터 로딩 작업에 로깅을 추가하면 추적에 용이 할거 같아요 : )

logger.info("Loading initial data..."); 처음
logger.info("Data loading completed."); 마지막

Comment on lines 37 to 57
public void run(String... args) throws Exception {
memberRepository.save(new Member("어드민", "[email protected]", "password", "ADMIN"));
memberRepository.save(new Member("브라운", "[email protected]", "password", "USER"));

Theme theme1 = themeRepository.save(new Theme("테마1", "테마1입니다."));
Theme theme2 = themeRepository.save(new Theme("테마2", "테마2입니다."));
Theme theme3 = themeRepository.save(new Theme("테마3", "테마3입니다."));

Time time1 = timeRepository.save(new Time("10:00"));
Time time2 = timeRepository.save(new Time("12:00"));
Time time3 = timeRepository.save(new Time("14:00"));
timeRepository.save(new Time("16:00"));
timeRepository.save(new Time("18:00"));
timeRepository.save(new Time("20:00"));

reservationRepository.save(new Reservation("브라운", "2024-03-01", time1, theme2));
reservationRepository.save(new Reservation("어드민", "2024-03-01", time1, theme1));
reservationRepository.save(new Reservation("어드민", "2024-03-01", time2, theme2));
reservationRepository.save(new Reservation("어드민", "2024-03-01", time3, theme3));
}
}

Choose a reason for hiding this comment

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

동일하게 추적을 높이기 위해 로깅을 추가하면 좋을거 같아요 : )

Comment on lines +71 to +72
List<WaitingWithRank> waitings = waitingRepository.findWaitingsWithRankByMemberId(loginMember.id());

Choose a reason for hiding this comment

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

반환값이 null 일 수도 있어 예외처리를 해주면 더욱 좋을거 같아요

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