[#7] 리펙토링 및 로직 수정 #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmihye
commented
Jan 22, 2025
Comment on lines
+44
to
+51
|
|
||
| //한국 시간대로 변환 | ||
| ZoneId koreaTimeZone = ZoneId.of("Asia/Seoul"); | ||
| ZonedDateTime koreaTime = now.atDate(ZonedDateTime.now().toLocalDate()).atZone(koreaTimeZone); | ||
|
|
||
| //ZonedDateTime에서 LocalTime 추출 | ||
| LocalTime koreaLocalTime = koreaTime.toLocalTime(); | ||
|
|
Member
Author
There was a problem hiding this comment.
요것도 한국시간으로 설정했던거같아서 없애고 싶은데 정확하지 않아서 일단,,,
Contributor
There was a problem hiding this comment.
그래도 많이 깔끔해졌네용 키키 시간 관련된 도메인을 하나 만들어서 빼는거 생각해봐도 좋을거같은데 같이 고민해봅시당
sss4920
reviewed
Jan 22, 2025
| // TODO 플랫폼마다 별도의 설정이 필요한 경우 사용 | ||
|
|
||
| // Android | ||
| public AndroidConfig TokenAndroidConfig(FCMPushRequestDto request) { |
Contributor
There was a problem hiding this comment.
아 오랜만에 이쪽 코드를 봐서 그런지 헷갈리는데 안드로이드 부분 이거 없애도 됐는지 기억이 잘안나네요..ㅋㅋ 혹시 이거 관련해서 테스트 해보셨을까요??
Member
Author
There was a problem hiding this comment.
요거 생각해보니 있어야할것같아서 다시 추가합니다...ㅎㅎ
Comment on lines
+44
to
+51
|
|
||
| //한국 시간대로 변환 | ||
| ZoneId koreaTimeZone = ZoneId.of("Asia/Seoul"); | ||
| ZonedDateTime koreaTime = now.atDate(ZonedDateTime.now().toLocalDate()).atZone(koreaTimeZone); | ||
|
|
||
| //ZonedDateTime에서 LocalTime 추출 | ||
| LocalTime koreaLocalTime = koreaTime.toLocalTime(); | ||
|
|
Contributor
There was a problem hiding this comment.
그래도 많이 깔끔해졌네용 키키 시간 관련된 도메인을 하나 만들어서 빼는거 생각해봐도 좋을거같은데 같이 고민해봅시당
| String title = ""; | ||
| String body = ""; | ||
|
|
||
| switch (randomNumber) { |
Contributor
There was a problem hiding this comment.
보니까 스위치 문에서 title body가 엄청 겹쳐서 만들어지네욤 흠냐 ㅋㅋ
지금 드는 생각은 title, body를 담는 도메인을 만들어서 코드를 수정할 수 있을거같은데 저도 고민 좀 해보겠숩니당 ㅋㅋ
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚩 관련 이슈
📋 구현 기능 명세
📌 PR Point
무슨 이유로 어떻게 코드를 변경했는지
1분마다 모든 리마인더 데이터를 가져와서 비교하는 로직보다 쿼리문에서 걸러서 가져오는것이 더 좋을것같아 그렇게 변경했습니다.
어떤 부분에 리뷰어가 집중해야 하는지
지금 remindDates가 배열을 String으로 저장하고 있어 natvieQuery를 사용했는데 날짜배열을 테이블 분리해야할지는 고민해봐야할것같습니다...흠..
개발하면서 어떤 점이 궁금했는지
reminder를 가져와서 user 처리할때 지연로딩 오류로 일단 EAGER로 바꿨는데,
이건 쿼리에서 user 정보까지 한번에 가져오는 식으로 추후에 변경하는게 좋을지
지금 이대로 가도 괜찮을지 고민이네요