Skip to content

Step2 #288

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

Closed
wants to merge 9 commits into from
Closed

Step2 #288

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions README.md
Copy link

Choose a reason for hiding this comment

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

아직 많이 미숙하지만 tdd로 미션을 진행하다 보니 개별적으로 작성한 클래스들을 마지막에 합치는 과정에서 많은 어려움이 있었습니다.
처음부터 큰 그림 로직 설계를 한 다음 세부 클래스 tdd를 진행해야 할까요??

tdd 자체의 어려움과 새로 접한 도메인의 어려움이 섞여서 더 어려우셨을 것 같습니다.
먼저 관련해서 읽어보시면 도움이 될 것 같은 블로그를 공유드려요. 주요 키워드는 Inside Out 과 Outside In 입니다.

개별적으로 클래스를 작성한 후 합치는 방법은 초기 설계가 잘 되어 있거나 우리가 잘 아는 도메인일 때 더 유용할 수 있다고 생각합니다. 말씀하신 것처럼, 처음에 큰 로직(전체 설계)을 고민해보신 후 tdd로 접근하시는 것이 더 수월하셨을 것 같아요. :)

Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
# 학습 관리 시스템(Learning Management System)
## 질문 삭제하기 요구사항
## 2단계 - 수강신청(도메인 모델)
### 수강 신청 기능 요구사항
* 과정(Course)은 기수 단위로 운영하며, 여러 개의 강의(Session)를 가질 수 있다.
* 강의는 시작일과 종료일을 가진다.
* 강의는 강의 커버 이미지 정보를 가진다.
* 이미지 크기는 1MB 이하여야 한다.
* 이미지 타입은 gif, jpg(jpeg 포함),, png, svg만 허용한다.
* 이미지의 width는 300픽셀, height는 200픽셀 이상이어야 하며, width와 height의 비율은 3:2여야 한다.
* 강의는 무료 강의와 유료 강의로 나뉜다.
* 무료 강의는 최대 수강 인원 제한이 없다.
* 유료 강의는 강의 최대 수강 인원을 초과할 수 없다.
* 유료 강의는 수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다.
* 강의 상태는 준비중, 모집중, 종료 3가지 상태를 가진다.
* 강의 수강신청은 강의 상태가 모집중일 때만 가능하다.
* 유료 강의의 경우 결제는 이미 완료한 것으로 가정하고 이후 과정을 구현한다.
* 결제를 완료한 결제 정보는 payments 모듈을 통해 관리되며, 결제 정보는 Payment 객체에 담겨 반한된다.
---
* [x] 질문 데이터를 완전히 삭제하는 것이 아니라 데이터의 상태를 삭제 상태(deleted - boolean type)로 변경한다.
* [x] 로그인 사용자와 질문한 사람이 같은 경우 삭제 가능하다.
* [x] 답변이 없는 경우 삭제가 가능하다.
* [x] 답변을 삭제할 수 있다.
* [x] 질문자와 답변글의 모든 답변자가 같은 경우 삭제가 가능하다.
* [x] 질문을 삭제할 때 답변 또한 삭제해야 하며, 답변의 삭제 또한 삭제 상태(deleted)를 변경한다.
* [x] 질문과 답변 삭제 이력에 대한 정보를 DeleteHistory를 활용해 남긴다.
---
178 changes: 89 additions & 89 deletions gradlew.bat
Original file line number Diff line number Diff line change
@@ -1,89 +1,89 @@
@rem
@rem Copyright 2015 the original author or authors.
@rem
@rem Licensed under the Apache License, Version 2.0 (the "License");
@rem you may not use this file except in compliance with the License.
@rem You may obtain a copy of the License at
@rem
@rem https://www.apache.org/licenses/LICENSE-2.0
@rem
@rem Unless required by applicable law or agreed to in writing, software
@rem distributed under the License is distributed on an "AS IS" BASIS,
@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@rem See the License for the specific language governing permissions and
@rem limitations under the License.
@rem

@if "%DEBUG%" == "" @echo off
@rem ##########################################################################
@rem
@rem Gradle startup script for Windows
@rem
@rem ##########################################################################

@rem Set local scope for the variables with windows NT shell
if "%OS%"=="Windows_NT" setlocal

set DIRNAME=%~dp0
if "%DIRNAME%" == "" set DIRNAME=.
set APP_BASE_NAME=%~n0
set APP_HOME=%DIRNAME%

@rem Resolve any "." and ".." in APP_HOME to make it shorter.
for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi

@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m"

@rem Find java.exe
if defined JAVA_HOME goto findJavaFromJavaHome

set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto execute

echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
echo.
echo Please set the JAVA_HOME variable in your environment to match the
echo location of your Java installation.

goto fail

:findJavaFromJavaHome
set JAVA_HOME=%JAVA_HOME:"=%
set JAVA_EXE=%JAVA_HOME%/bin/java.exe

if exist "%JAVA_EXE%" goto execute

echo.
echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME%
echo.
echo Please set the JAVA_HOME variable in your environment to match the
echo location of your Java installation.

goto fail

:execute
@rem Setup the command line

set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar


@rem Execute Gradle
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*

:end
@rem End local scope for the variables with windows NT shell
if "%ERRORLEVEL%"=="0" goto mainEnd

:fail
rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of
rem the _cmd.exe /c_ return code!
if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
exit /b 1

:mainEnd
if "%OS%"=="Windows_NT" endlocal

:omega
@rem
@rem Copyright 2015 the original author or authors.
@rem
@rem Licensed under the Apache License, Version 2.0 (the "License");
@rem you may not use this file except in compliance with the License.
@rem You may obtain a copy of the License at
@rem
@rem https://www.apache.org/licenses/LICENSE-2.0
@rem
@rem Unless required by applicable law or agreed to in writing, software
@rem distributed under the License is distributed on an "AS IS" BASIS,
@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@rem See the License for the specific language governing permissions and
@rem limitations under the License.
@rem

@if "%DEBUG%" == "" @echo off
@rem ##########################################################################
@rem
@rem Gradle startup script for Windows
@rem
@rem ##########################################################################

@rem Set local scope for the variables with windows NT shell
if "%OS%"=="Windows_NT" setlocal

set DIRNAME=%~dp0
if "%DIRNAME%" == "" set DIRNAME=.
set APP_BASE_NAME=%~n0
set APP_HOME=%DIRNAME%

@rem Resolve any "." and ".." in APP_HOME to make it shorter.
for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi

@rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m"

@rem Find java.exe
if defined JAVA_HOME goto findJavaFromJavaHome

set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto execute

echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
echo.
echo Please set the JAVA_HOME variable in your environment to match the
echo location of your Java installation.

goto fail

:findJavaFromJavaHome
set JAVA_HOME=%JAVA_HOME:"=%
set JAVA_EXE=%JAVA_HOME%/bin/java.exe

if exist "%JAVA_EXE%" goto execute

echo.
echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME%
echo.
echo Please set the JAVA_HOME variable in your environment to match the
echo location of your Java installation.

goto fail

:execute
@rem Setup the command line

set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar


@rem Execute Gradle
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*

:end
@rem End local scope for the variables with windows NT shell
if "%ERRORLEVEL%"=="0" goto mainEnd

:fail
rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of
rem the _cmd.exe /c_ return code!
if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
exit /b 1

:mainEnd
if "%OS%"=="Windows_NT" endlocal

:omega
11 changes: 11 additions & 0 deletions src/main/java/nextstep/courses/domain/Course.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nextstep.courses.domain;

import java.time.LocalDateTime;
import java.util.List;

public class Course {
private Long id;
Expand All @@ -13,9 +14,15 @@ public class Course {

private LocalDateTime updatedAt;

private List<Session> sessions;

public Course() {
}

public Course(List<Session> sessions) {
Copy link

Choose a reason for hiding this comment

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

모든 인자를 받는 생성자에도 List<Session> sessions를 추가하는 건 어떨까요? :)
현재 이 생성자는 테스트에서만 사용되어서, 테스트만을 위해 생성자를 추가한 것처럼 보이기도 하네요.

this.sessions = sessions;
}

public Course(String title, Long creatorId) {
this(0L, title, creatorId, LocalDateTime.now(), null);
}
Expand All @@ -40,6 +47,10 @@ public LocalDateTime getCreatedAt() {
return createdAt;
}

public List<Session> getSessions() {
return sessions;
}

@Override
public String toString() {
return "Course{" +
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/nextstep/courses/domain/Enrolment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package nextstep.courses.domain;

public class Enrolment {
private final Session session;

public Enrolment(Session session) {
this.session = session;
}

public void register() {
validateSessionStatus();
session.addStudent();
}

private void validateSessionStatus() {
Copy link

Choose a reason for hiding this comment

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

session에 사용자를 등록할 때(addStudent) 검증하는 것인데 session 내부에서 상태를 가지고 있네요. session#addStudent 메서드 내에서 검증해도 괜찮지 않을까요?

session에서 여러개의 public 메서드를 제공할 때 외부 클래스(또는 다른 개발자)가 기대한 순서대로 호출할 것이라고 가정하는 것은 위험합니다. 다른 클래스는 구현의 세부 사항을 모르고, 몰라야 합니다. 따라서, 하나의 public 메서드를 호출했을 때 온전하게 기대한 동작을 할 수 있도록 구현해보시면 좋겠습니다.

if (!session.isSessionStatus().isSessionResult()) {
throw new IllegalArgumentException();
}
}
}
7 changes: 7 additions & 0 deletions src/main/java/nextstep/courses/domain/FreeSession.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package nextstep.courses.domain;

public class FreeSession extends Session {
public FreeSession(int studentCount, int tuition) {
super(studentCount, tuition);
}
}
51 changes: 51 additions & 0 deletions src/main/java/nextstep/courses/domain/Image.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package nextstep.courses.domain;

public class Image {
private String type;
private int width;
private int height;

public Image() {
}

public Image(String type, int width, int height) {
validateImage(type, width, height);
this.type = type;
this.width = width;
this.height = height;
}

public String getType() {
return type;
}

public int getWidth() {
return width;
}

public int getHeight() {
return height;
}

private static void validateImage(String type, int width, int height) {
if (width * height > 1024 * 1024) {
Copy link

Choose a reason for hiding this comment

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

1 픽셀을 표현하기 위해 사용되는 바이트 수가 많아진다면, 크기만으로 검증하는 건 어려울 것 같아요.
사이즈 필드를 명시적으로 추가하고 검증에 사용하는 건 어떨까요?

throw new IllegalArgumentException("이미지 크기는 1MB 이하여야 한다.");
}

if (!(type.contains("gif") || type.contains("jpg") || type.contains("jpeg") || type.contains("png") || type.contains("svg"))) {
Copy link

Choose a reason for hiding this comment

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

타입 검증 좋습니다. 👍
우리가 관리하고 싶은 type이 정해져 있으니, enum 을 이용하는 방법도 사용해 볼 수 있겠네요!

throw new IllegalArgumentException("이미지 타입은 gif, jpg(jpeg 포함),, png, svg만 허용한다.");
}

if (width < 300) {
Copy link

Choose a reason for hiding this comment

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

width, height를 관리하는 사이즈 클래스를 만들어보는 건 어떨까요?

throw new IllegalArgumentException("이미지 넓이는 300px 이상이여야 한다.");
}

if (height < 200) {
throw new IllegalArgumentException("이미지 높이는 200px 이상이여야 한다.");
}

if (width / height != 3 / 2) {
throw new IllegalArgumentException("넓이와 높이의 비율은 3:2여야 한다.");
}
}
}
32 changes: 32 additions & 0 deletions src/main/java/nextstep/courses/domain/PaidSession.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package nextstep.courses.domain;

public class PaidSession extends Session {
Copy link

Choose a reason for hiding this comment

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

유료 세션과 무료 세션을 구분하신 이유가 있을까요?
Enrollment라는 클래스가 있으니, 수강신청 단계에서 검증해야 하는 부분을 Enrollment가 맡는다면 세션을 하나로도 관리할 수 있지 않을까요?

private static final int PAID_SESSION_TUITION_FEE = 10000;
private static final int MAX_SESSION_STUDENT_COUNT = 5;

public PaidSession(int tuition) {
super(tuition);
validateTuitionFee(tuition);
}

public PaidSession(SessionStatus sessionStatus) {
super(sessionStatus);
}

public PaidSession(int studentCount, int tuition) {
super(studentCount, tuition);
validateStudentCount(studentCount);
Copy link

Choose a reason for hiding this comment

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

요구사항에 대한 요구사항 중 아래 내용을 구현하신 것으로 이해했습니다.

  • 유료 강의는 강의 최대 수강 인원을 초과할 수 없다.

최대 수강 인원은 Session에서 관리해야 하는 값으로 보여요! 유료 강의에서만 최대 수강 인원을 관리한다면, 별도의 필드를 추가할 필요가 있어 보입니다. 그리고 studentCount를 증가시킬 때 최대 수강 인원보다 적은지 검증해야 할 것으로 보이네요.
현재는 "최대 수강 인원"에 대한 "최댓값"을 5로 관리하고, 이를 검증한 것으로 보입니다.

}

private void validateTuitionFee(int tuition) {
if (tuition != PAID_SESSION_TUITION_FEE) {
throw new IllegalArgumentException();
}
}

private void validateStudentCount(int studentCount) {
if (studentCount > MAX_SESSION_STUDENT_COUNT) {
throw new IllegalArgumentException();
}
}
}
Loading