-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR] 타임박스 도메인 - 엔티티 분리 / 도메인 구현 #198
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
Changes from all commits
7877571
300ea82
142f543
bba9736
062e8ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package com.debatetimer.domain.customize; | ||
|
|
||
| import com.debatetimer.exception.custom.DTClientErrorException; | ||
| import com.debatetimer.exception.errorcode.ClientErrorCode; | ||
| import org.springframework.lang.Nullable; | ||
|
|
||
| public abstract class CustomizeTimeBoxDomain { | ||
|
|
||
| public static final int SPEECH_TYPE_MAX_LENGTH = 10; | ||
| public static final int SPEAKER_MAX_LENGTH = 5; | ||
|
|
||
| private final Stance stance; | ||
|
|
||
| private final String speechType; | ||
|
|
||
| @Nullable | ||
| private final String speaker; | ||
|
|
||
| protected CustomizeTimeBoxDomain(Stance stance, String speechType, @Nullable String speaker) { | ||
| validateStance(stance); | ||
| validateSpeechType(speechType); | ||
| validateSpeaker(speaker); | ||
|
|
||
| this.stance = stance; | ||
| this.speechType = speechType; | ||
| this.speaker = speaker; | ||
| } | ||
|
|
||
| private void validateStance(Stance stance) { | ||
| if (stance == null || !isValidStance(stance)) { | ||
|
Contributor
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. isValidStance가 지금 not조건으로만 쓰이는 것 같은데 차라리 isInvalidStance로 사용하는 건 어떤가요?
Member
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. 단순하게 말 만 적어서 잘 모르겠는데, 아래와 같은 로직을 말하는 건가요? private void validateStance(Stance stance) {
if (stance != null && isValidStance(stance)) {
return;
}
throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_STANCE);
}
Contributor
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. 이걸 말한 거긴 합니다.
Member
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. 구현하는 추상 메서드 입장에서는 |
||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_STANCE); | ||
| } | ||
| } | ||
|
|
||
| protected abstract boolean isValidStance(Stance stance); | ||
|
|
||
| private void validateSpeechType(String speechType) { | ||
|
Contributor
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. [받아들여지지 않아도 꿋꿋히 해보는 제안] Table에서도 마찬가지였는데 값 객체로 감싸는 건 싫은가요? SpeechType, Speaker 등등이요
Member
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.
딱 그만큼 깔끔해지기를 원했습니다.
네 |
||
| if (speechType == null || speechType.isBlank() || speechType.length() > SPEECH_TYPE_MAX_LENGTH) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_SPEECH_TYPE_LENGTH); | ||
| } | ||
| } | ||
|
|
||
| private void validateSpeaker(String speaker) { | ||
| if (speaker != null && speaker.length() > SPEAKER_MAX_LENGTH) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_SPEAKER_LENGTH); | ||
| } | ||
| } | ||
|
|
||
| public final Stance getStance() { | ||
| return stance; | ||
| } | ||
|
|
||
| public final String getSpeechType() { | ||
| return speechType; | ||
| } | ||
|
|
||
| @Nullable | ||
| public final String getSpeaker() { | ||
|
Contributor
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. 커꼼(커찬 꼼꼼이라는 뜻) 👍 |
||
| return speaker; | ||
| } | ||
|
|
||
| public abstract CustomizeBoxType getBoxType(); | ||
|
|
||
| @Nullable | ||
| public abstract Integer getTime(); | ||
|
|
||
| @Nullable | ||
| public abstract Integer getTimePerTeam(); | ||
|
|
||
| @Nullable | ||
| public abstract Integer getTimePerSpeaking(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package com.debatetimer.domain.customize; | ||
|
|
||
| import com.debatetimer.exception.custom.DTClientErrorException; | ||
| import com.debatetimer.exception.errorcode.ClientErrorCode; | ||
| import org.springframework.lang.Nullable; | ||
|
|
||
| public final class NormalTimeBoxDomain extends CustomizeTimeBoxDomain { | ||
|
|
||
| private final int time; | ||
|
|
||
| public NormalTimeBoxDomain(Stance stance, String speechType, @Nullable String speaker, Integer time) { | ||
| super(stance, speechType, speaker); | ||
|
|
||
| validateTime(time); | ||
| this.time = time; | ||
| } | ||
|
|
||
| private void validateTime(Integer time) { | ||
| if (time == null || time <= 0) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_TIME); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean isValidStance(Stance stance) { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public CustomizeBoxType getBoxType() { | ||
| return CustomizeBoxType.NORMAL; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getTime() { | ||
| return time; | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public Integer getTimePerTeam() { | ||
| return null; | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public Integer getTimePerSpeaking() { | ||
| return null; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,9 @@ public enum Stance { | |
| PROS, | ||
| CONS, | ||
| NEUTRAL, | ||
| ; | ||
|
|
||
| public boolean isNeutralStance() { | ||
| return this == NEUTRAL; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package com.debatetimer.domain.customize; | ||
|
|
||
| import com.debatetimer.exception.custom.DTClientErrorException; | ||
| import com.debatetimer.exception.errorcode.ClientErrorCode; | ||
| import org.springframework.lang.Nullable; | ||
|
|
||
| public class TimeBasedTimeBoxDomain extends CustomizeTimeBoxDomain { | ||
|
|
||
| private final int timePerTeam; | ||
|
|
||
| @Nullable | ||
| private final Integer timePerSpeaking; | ||
|
|
||
| public TimeBasedTimeBoxDomain(Stance stance, | ||
| String speechType, | ||
| @Nullable String speaker, | ||
| Integer timePerTeam, | ||
| @Nullable Integer timePerSpeaking) { | ||
| super(stance, speechType, speaker); | ||
|
|
||
| validateTimes(timePerTeam, timePerSpeaking); | ||
| this.timePerTeam = timePerTeam; | ||
| this.timePerSpeaking = timePerSpeaking; | ||
| } | ||
|
|
||
| private void validateTimes(Integer timePerTeam, Integer timePerSpeaking) { | ||
| validateTimePerTeam(timePerTeam); | ||
| validateTimePerSpeaking(timePerSpeaking); | ||
| if (timePerSpeaking != null && timePerTeam < timePerSpeaking) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BASED_TIME); | ||
| } | ||
| } | ||
|
|
||
| private void validateTimePerTeam(Integer time) { | ||
| if (time == null || time <= 0) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_TIME); | ||
| } | ||
| } | ||
|
|
||
| private void validateTimePerSpeaking(Integer timePerSpeaking) { | ||
| if (timePerSpeaking != null && timePerSpeaking <= 0) { | ||
| throw new DTClientErrorException(ClientErrorCode.INVALID_TIME_BOX_TIME); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean isValidStance(Stance stance) { | ||
| return stance.isNeutralStance(); | ||
| } | ||
|
|
||
| @Override | ||
| public CustomizeBoxType getBoxType() { | ||
| return CustomizeBoxType.TIME_BASED; | ||
| } | ||
|
|
||
| @Override | ||
| @Nullable | ||
| public Integer getTime() { | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getTimePerTeam() { | ||
| return timePerTeam; | ||
| } | ||
|
|
||
| @Override | ||
| @Nullable | ||
| public Integer getTimePerSpeaking() { | ||
| return timePerSpeaking; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| package com.debatetimer.domain.customize; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThatCode; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| import com.debatetimer.exception.custom.DTClientErrorException; | ||
| import com.debatetimer.exception.errorcode.ClientErrorCode; | ||
| import org.junit.jupiter.api.Nested; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.NullAndEmptySource; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| class CustomizeTimeBoxDomainTest { | ||
|
|
||
| @Nested | ||
| class ValidateStance { | ||
|
|
||
| @Test | ||
| void 발언_입장은_비어있을_수_없다() { | ||
| assertThatThrownBy(() -> new InheritedCustomizeTimeBoxDomain(null, "비토", "발언자")) | ||
|
Contributor
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. 이전 논의사항을 잘 반영해주신 테스트네요 굳 👍 |
||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_STANCE.getMessage()); | ||
| } | ||
leegwichan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @Test | ||
| void 발언_입장은_유효한_값이어야_한다() { | ||
| assertThatCode(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, "비토", "발언자")) | ||
| .doesNotThrowAnyException(); | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| class ValidateSpeechType { | ||
|
|
||
| @Test | ||
| void 발언_종류는_특정_글자를_초과할_수_없다() { | ||
| String speechType = "a".repeat(CustomizeTimeBoxDomain.SPEECH_TYPE_MAX_LENGTH + 1); | ||
|
|
||
| assertThatThrownBy(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, speechType, "비토")) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_SPEECH_TYPE_LENGTH.getMessage()); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @NullAndEmptySource | ||
| @ValueSource(strings = {" ", "\n\t"}) | ||
| void 발언_종류는_비어있을_수_없다(String emptySpeechType) { | ||
| assertThatThrownBy(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, emptySpeechType, "비토")) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_SPEECH_TYPE_LENGTH.getMessage()); | ||
| } | ||
|
|
||
| @Test | ||
| void 발언_종류는_특정_글자_이내이어야_한다() { | ||
| String speechType = "a".repeat(CustomizeTimeBoxDomain.SPEECH_TYPE_MAX_LENGTH); | ||
|
|
||
| assertThatCode(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, speechType, "비토")) | ||
| .doesNotThrowAnyException(); | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| class ValidateSpeaker { | ||
|
|
||
| @Test | ||
| void 발언자_이름은_특정_글자를_초과할_수_없다() { | ||
| String speaker = "a".repeat(CustomizeTimeBoxDomain.SPEAKER_MAX_LENGTH + 1); | ||
|
|
||
| assertThatThrownBy(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, "비토", speaker)) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_SPEAKER_LENGTH.getMessage()); | ||
| } | ||
|
|
||
| @Test | ||
| void 발언자_이름은_비어있을_수_있다() { | ||
| assertThatCode(() -> new InheritedCustomizeTimeBoxDomain(Stance.PROS, "비토", null)) | ||
| .doesNotThrowAnyException(); | ||
| } | ||
| } | ||
|
|
||
| static class InheritedCustomizeTimeBoxDomain extends CustomizeTimeBoxDomain { | ||
|
|
||
| protected InheritedCustomizeTimeBoxDomain(Stance stance, String speechType, String speaker) { | ||
| super(stance, speechType, speaker); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean isValidStance(Stance stance) { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public CustomizeBoxType getBoxType() { | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getTime() { | ||
| return 0; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getTimePerTeam() { | ||
| return 0; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer getTimePerSpeaking() { | ||
| return 0; | ||
| } | ||
| } | ||
leegwichan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package com.debatetimer.domain.customize; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThatCode; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| import com.debatetimer.exception.custom.DTClientErrorException; | ||
| import com.debatetimer.exception.errorcode.ClientErrorCode; | ||
| import org.junit.jupiter.api.Nested; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class NormalTimeBoxDomainTest { | ||
|
|
||
| @Nested | ||
| class ValidateTime { | ||
|
|
||
| @Test | ||
| void 시간은_0보다_커야_한다() { | ||
| Integer time = 0; | ||
|
|
||
| assertThatThrownBy(() -> new NormalTimeBoxDomain(Stance.PROS, "비토", null, time)) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_TIME.getMessage()); | ||
| } | ||
|
|
||
| @Test | ||
| void 시간은_비어있지_않아야_한다() { | ||
| Integer time = null; | ||
|
|
||
| assertThatThrownBy(() -> new NormalTimeBoxDomain(Stance.PROS, "비토", null, time)) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.INVALID_TIME_BOX_TIME.getMessage()); | ||
| } | ||
|
|
||
| @Test | ||
| void 시간은_양수여야_한다() { | ||
| Integer time = 1; | ||
|
|
||
| assertThatCode(() -> new NormalTimeBoxDomain(Stance.PROS, "비토", null, time)) | ||
| .doesNotThrowAnyException(); | ||
| } | ||
| } | ||
| } |
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.
[제안]
Nullable은 명세차원이라 적어준 것이죠? Stance와 speechType은 NotNull이면 안적어준 이유가 있는지 궁금하네요! 밑에 검증해서??!
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.
이거 어떤 것은 Nullable 하고 어떤 것은 not null 이어서 코드 작업할 때 쉽게 파악하라고 적어두었습니다.
기본적으로 Not null 하게 코드를 짠다는 것이 서로 코드 스타일로 자리잡혀있다고 생각해서 따로 명시해주지는 않았습니다.