refactor: 예약 도메인 관련 객체들 생성 #796
Merged
Conversation
This file contains 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
...main/java/com/woowacourse/zzimkkong/exception/reservation/IllegalTimeUnitValueException.java
Show resolved
Hide resolved
backend/src/main/java/com/woowacourse/zzimkkong/domain/TimeUnit.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/woowacourse/zzimkkong/domain/TimeUnit.java
Outdated
Show resolved
Hide resolved
xrabcde
requested changes
May 8, 2022
backend/src/main/java/com/woowacourse/zzimkkong/domain/Reservation.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/woowacourse/zzimkkong/domain/Member.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/woowacourse/zzimkkong/domain/Map.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/woowacourse/zzimkkong/domain/TimeSlot.java
Outdated
Show resolved
Hide resolved
Comment on lines
+41
to
+51
| if (TimeUnit.cannotDivideByMinimumTimeUnit(startTime.getMinute())) { | ||
| throw new IllegalTimeUnitValueException(startTime.getMinute()); | ||
| } | ||
|
|
||
| if (TimeUnit.cannotDivideByMinimumTimeUnit(endTime.getMinute())) { | ||
| throw new IllegalTimeUnitValueException(endTime.getMinute()); | ||
| } | ||
| } | ||
|
|
||
| public boolean isNotDivisibleBy(final TimeUnit timeUnit) { | ||
| return !(timeUnit.canDivide(startTime) && timeUnit.canDivide(endTime)); |
There was a problem hiding this comment.
이거 왜 어디서는 cannotDivideBy 를 쓰고 어디서는 isNotDivisibleBy 를 쓰는건가유...
통일하면 좋겠는걸~..~
There was a problem hiding this comment.
말한대로 참 애매한 부분이긴해서...
더군다나 cannotDivideByMinimumTimeUnit가 static method라서 더 이상하게 보였을 수 있겠네요.
저는 나름 아래와 같은 기준으로 나눠서 작명을 했습니다.
# 호출자 기준 능동적인 상황이냐 수동적인 상황이냐가 기준
# 능동은 cannot*, 수동은 is*
# 받는 parameter가 나눔의 대상 (능동)
e.g. TimeUnit.cannotDivideByMinimumTimeUnit(minutes)
즉, TimeUnit 입장에서 MINIMUM_TIME_UNIT으로 minutes를 나눴을 때, 나누어 떨어지는가?
# 받는 parameter가 나눔의 주체 (수동)
e.g. timeSlot.isNotDivisibleBy(timeUnit)
즉, timeSlot이 timeUnit으로 나누어 떨어지는가?
이렇게라도 구분하지않으면, 시간 관련 메서드명이 규칙없이 더 혼란스러워 질 것 같아서 나름 구분해서 작명했습니다.
혹시 더 좋은 방안이 있다면 제안 해주세요 ㅠ
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋ 안그래도 빡센 타임존에 네이밍까지 고민하시느라 고생이 많았네유...
나름의 기준이 있었군요 ㅋㅋㅋ 이렇게 의도하신 바를 공유만 잘 해주신다면 저는 상관없습니다아ㅏㅏ~~
...main/java/com/woowacourse/zzimkkong/exception/reservation/IllegalTimeUnitValueException.java
Show resolved
Hide resolved
backend/src/test/java/com/woowacourse/zzimkkong/domain/ReservationTimeTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/com/woowacourse/zzimkkong/domain/TimeSlotTest.java
Outdated
Show resolved
Hide resolved
cache ConcurrentHashMap, code reformatting & refactoring
xrabcde
approved these changes
May 23, 2022
Comment on lines
+41
to
+51
| if (TimeUnit.cannotDivideByMinimumTimeUnit(startTime.getMinute())) { | ||
| throw new IllegalTimeUnitValueException(startTime.getMinute()); | ||
| } | ||
|
|
||
| if (TimeUnit.cannotDivideByMinimumTimeUnit(endTime.getMinute())) { | ||
| throw new IllegalTimeUnitValueException(endTime.getMinute()); | ||
| } | ||
| } | ||
|
|
||
| public boolean isNotDivisibleBy(final TimeUnit timeUnit) { | ||
| return !(timeUnit.canDivide(startTime) && timeUnit.canDivide(endTime)); |
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋ 안그래도 빡센 타임존에 네이밍까지 고민하시느라 고생이 많았네유...
나름의 기준이 있었군요 ㅋㅋㅋ 이렇게 의도하신 바를 공유만 잘 해주신다면 저는 상관없습니다아ㅏㅏ~~
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.
구현 기능
새로 생긴 예약 관련 클래스들
ReservationTime: 예약 시간에 대한 객체TimeSlot: 시작~끝 시간을 나타내는 객체TimeUnit: 찜꽁 서비스에서 쓰는 시간 단위 (5분, 10분, ...)논의하고 싶은 내용
Reservaiton과Setting에 @Embedded 되어있음TimeSlot을ReservationTime의 컴포넌트(@embeddable)로 쓰는 멋진 그림을 그렸었지만, 예약 시간은reservation테이블 설계 상LocalDateTime으로 되어있기 때문에 그렇게 하지 못했음. 코드를 보면 이해할 것.#746 관련 리팩터링 입니다. 다른 부분에서도 추가적으로 리팩터링 진행 가능한 부분이 있을 수도있어서 Close는 하지 않겠습니다.
그리고 이 PR은 #793 이 머지된 이후에 머지되어야 합니다.
이전 PR에 얹어서 작업한거라... files changed보고 놀라지 마세요.