feat : Step3 수강신청(DB 적용) #201

qkrxodud · 2023-06-18T06:52:42Z

step2를 삭제하고 진행 했어야 됬는데, 삭제하지 않고 작업을 하다 꼬였네요. 😢😢😢
step4 부터는 커밋단위를 좀 더 나눠서 진행하도록 하겠습니다.

javajigi

도메인 객체와 DB 매핑하느라 노력한 흔적이 보이네요. 👍
DB 매핑 객체를 도메인 객체로 변환하는 부분도 보이네요.
도메인 객체와 DB 매핑 객체 관련해 피드백 남겨봤어요.

javajigi · 2023-06-19T03:49:00Z

src/main/java/nextstep/courses/domain/Session.java

        this.id = id;
+        this.title = title;
        this.students = new Students();
        this.imageFile = new ImgFile(imageFile);
        sessionDuration = new SessionDuration(startedAt, endedAt);


주생성자를 호출해 구현하는 습관을 들이면 어떨까?

javajigi · 2023-06-19T03:50:48Z

src/main/java/nextstep/courses/domain/Session.java

+    public void changeStudents(Students students) {
+        this.students = students;
+    }
+
    public void removeStudent(NsUser nsUser) {


메서드 이름을 putStudent, removeStudent과 정하면 자료구조에 데이터를 추가하고 제거하는 느낌이 든다.
이 보다는 도메인 의도가 드러나도록 enroll(수강신청), cancel(수강취소)와 같은 이름으로 구현하면 어떨까?

javajigi · 2023-06-19T03:51:20Z

src/main/java/nextstep/courses/domain/SessionDuration.java

        checkCourseDuration(startedAt, endedAt);
        this.startedAt = startedAt;
        this.endedAt = endedAt;
    }

-    void checkCourseDuration(LocalDate startedAt, LocalDate endedAt) {
+    void checkCourseDuration(LocalDateTime startedAt, LocalDateTime endedAt) {


Suggested change

void checkCourseDuration(LocalDateTime startedAt, LocalDateTime endedAt) {

private void checkCourseDuration(LocalDateTime startedAt, LocalDateTime endedAt) {

private으로 구현해도 되지 않을까?

javajigi · 2023-06-19T03:52:31Z

src/main/java/nextstep/courses/domain/SessionRepository.java

+    int update(Session session);
+
+    int delete(Long id);
+    Optional<List<Session>> findBySessionIds(List<Long> sessionIds);


Suggested change

Optional<List<Session>> findBySessionIds(List<Long> sessionIds);

List<Session> findBySessionIds(List<Long> sessionIds);

값이 없는 경우 빈 List 콜렉션을 반환하면 되기 때문에 콜렉션의 경우 굳이 Optional로 구현할 필요가 있을까?

javajigi · 2023-06-19T03:52:48Z

src/main/java/nextstep/courses/domain/SessionUserMapping.java

+
+import java.time.LocalDateTime;
+
+public class SessionUserMapping {


javajigi · 2023-06-19T03:53:16Z

src/main/java/nextstep/courses/domain/SessionUserMapping.java

+
+import java.time.LocalDateTime;
+
+public class SessionUserMapping {


클래스명에 굳이 Mapping이 필요할까?
SessionUser와 같이 구현해도 되지 않을까?

javajigi · 2023-06-19T03:53:56Z

src/main/java/nextstep/courses/domain/SessionUserMapping.java

+import java.time.LocalDateTime;
+
+public class SessionUserMapping {
+    private Long id;


Suggested change

private Long id;

private final Long id;

가능하면 인스턴스 변수의 경우 final로 구현하고, 값을 변경해야 하는 경우에만 final을 제거하면 어떨까?

javajigi · 2023-06-19T03:55:38Z

src/main/java/nextstep/courses/domain/Students.java

-    private final Map<Long, NsUser> studentsMap = new HashMap<>();
+    private Map<Long, NsUser> studentsMap = new HashMap<>();
+
+    private List<NsUser> users = new ArrayList<>();


Suggested change

private List<NsUser> users = new ArrayList<>();

private List<SessionUserMapping> users = new ArrayList<>();

수강신청한 학생의 목록은 SessionUserMapping을 가지는 것은 어떨까?

javajigi · 2023-06-19T03:56:55Z

src/main/java/nextstep/courses/service/SessionService.java

+
+        return session;
+    }
+


수강신청 기능도 있어야 하지 않을까?

javajigi

피드백 반영 잘 했네요. 💯
소소한 피드백 몇 개 남겼는데요.
다음 단계 진행할 때 함께 반영해 보세요

javajigi · 2023-06-22T22:31:28Z

src/main/java/nextstep/courses/domain/SessionUsers.java

+import java.util.stream.Collectors;
+
+public class SessionUsers {
+    List<SessionUser> sessionUserMappingList;


Suggested change

List<SessionUser> sessionUserMappingList;

private final List<SessionUser> sessionUserMappingList;

javajigi · 2023-06-22T22:31:51Z

src/main/java/nextstep/courses/domain/Sessions.java

    }

-    public void putEntity(Session session) {
-        if (Objects.isNull(session)) {
+    public void putEntity(SessionUser sessionUser) {


javajigi · 2023-06-22T22:32:49Z

src/main/java/nextstep/courses/service/UserService.java

+
+
+@Service
+public class UserService {


UserService가 아니라 SessionService여야 하지 않을까?

feat : Step3 수강신청(DB 적용)

5f92ee8

javajigi requested changes Jun 19, 2023

View reviewed changes

박태영 added 8 commits June 19, 2023 20:55

*refactor : 수강신청, 수강신청 취소 함수명 변경

d341d8c

*refactor : 내부객체에서만 함수 사용 private 선언

61296c6

*refactor : 클래스명 수정 SeesionUserMapping -> SeesionUser

6de3c8f

*refactor : 클래스명 수정 SeesionUserMapping -> SeesionUser

ed0f018

*refactor : 빈컬렉션 반환으로, 불필요한 Optional 제거

3c24b81

*feat : 수강신청 기능 추가

1ec82f7

*refactor : 수강신청, 수강신청 취소 함수명 변경

2e8e4d5

*refactor : 유저의 강의(신청, 삭제, 수정)

02cbbbe

javajigi approved these changes Jun 22, 2023

View reviewed changes

javajigi merged commit c2b1fbd into next-step:qkrxodud Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat : Step3 수강신청(DB 적용) #201

feat : Step3 수강신청(DB 적용) #201

qkrxodud commented Jun 18, 2023

javajigi left a comment

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi Jun 19, 2023

javajigi left a comment

javajigi Jun 22, 2023

javajigi Jun 22, 2023

javajigi Jun 22, 2023

	void checkCourseDuration(LocalDateTime startedAt, LocalDateTime endedAt) {
	private void checkCourseDuration(LocalDateTime startedAt, LocalDateTime endedAt) {

	Optional<List<Session>> findBySessionIds(List<Long> sessionIds);
	List<Session> findBySessionIds(List<Long> sessionIds);


		import java.time.LocalDateTime;

		public class SessionUserMapping {

	private List<NsUser> users = new ArrayList<>();
	private List<SessionUserMapping> users = new ArrayList<>();

	List<SessionUser> sessionUserMappingList;
	private final List<SessionUser> sessionUserMappingList;

feat : Step3 수강신청(DB 적용) #201

feat : Step3 수강신청(DB 적용) #201

Conversation

qkrxodud commented Jun 18, 2023

javajigi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javajigi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment