[2단계 - 출석 다시 구현하기] 시소(이경민) 미션 제출합니다. #149

egaeng09 · 2025-03-02T20:11:27Z

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?
프롤로그에 셀프 체크를 작성했나요?

객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?

1~5점 중에서 선택해주세요.

선택한 점수의 이유를 적어주세요.

이전 STEP에서 한 객체에 너무 많은 책임을 몰아주었다고 판단하여, 여러 객체를 나누어 각 객체들이 주체적으로 있을 수 있도록 책임을 분산하려 노력하였습니다.

또 이전보다 객체지향 생활체조 요구사항을 지키기 위해 depth 2 이상 금지나 메서드 10라인 이하, 적절한 네이밍 등을 의식하고 지키고자 노력하였습니다. 줄여쓰지도 않았고 일급 컬렉션도 활용했습니다!

다만 적절하게 원시값을 포장할만한 부분을 찾지 못하였고, 엔티티를 작게 유지... 했는지에 대한 확신이 없어 이와 같은 점수를 선택하게 되었습니다.

어떤 부분에 집중하여 리뷰해야 할까요?

안녕하세요 제이! 벌써 연휴 마지막 날이네요 ㅜ ㅜ 연휴 잘 보내고 계신가요?
이번 STEP 2에서는 진행하며 다음과 같은 궁금증들이 생겼어요!

지난 스텝 1 당시 TDD 사이클을 진행하다가도 어느 순간 선 테스트 작성을 잊고 기능 작성을 하여 테스트를 놓친 메서드가 존재했어요. 이를 방지하기 위해 TDD 사이클을 의식적으로 진행하여 테스트를 빠뜨리지 않도록 이전보다 신경써보려 노력했어요.
다만 이번에 이와 관련해서 진행 중 헷갈리는 부분이 있었어요.

일단 TDD 사이클을 실패 테스트 작성 > 성공하도록 비즈니스 로직 작성 > 성공 후 리팩터링 과정으로 진행했어요.
실패 테스트에 대한 기능을 크게 구현하고 이를 리펙터링하여 여러 작은 기능으로 분리하였는데, 이 때 메서드를 총 몇 가지로 분리할 것이라고 계획을 잡고 세부 메서드들에 대한 테스트를 먼저 작성 후 메서드를 분리하는 것이 맞는 순서인지, 아니면 메서드를 일단 분리한 후 각 메서드에 대한 테스트를 추가로 작성하는 것이 맞는 순서인지 헷갈려요! (분리 계획 잡고 계획에 맞게 테스트 작성 후 메서드 분리 vs 메서드 분리 후 각 분리된 메서드에 대한 테스트)

에러 메시지를 어떻게 관리해야 할까요? 저는 전체적인 예외 처리를 도메인에서 진행하도록 구현했어요. 그런데 진행하다보니 문제점이 있더라구요. View의 요구사항 변경으로 에러 메시지 내용만 달라지게 되면 Domain 코드에 변경이 일어나요. 예를 들면 12월 4일 토요일은 등교일이 아닙니다. 와 같은 경우에 출력 형식이 변경되면 말이죠. 이렇게 바라보니 예외 메시지는 View에서 부분인 것 같은데, Constants나 Enum 클래스로 따로 관리하고 패키지를 global 내부에 두어야 할지... 😿
지금의 로직 중 일부가 비효율적이지 않은가? 하는 생각이 들었어요. 예를 들면

AttendanceDate originalAttendanceDate = attendanceBook.findAttendanceDateByNameAndDate(name, date);
attendanceBook.edit(name, date, `enterAttendanceTimeForEdit());

다음과 같은 코드를 보게 되면 findAttendanceDateByNameAndDate를 통해 해당 크루에 대한 기록이 존재하는지 확인하구요,
edit도 이름을 통해 크루에 대한 출석 기록 수정이 이루어지는데, 그 내부는 모두 이름을 통해 크루가 있는지 확인을 한단 말이죠?
그러면 기존 findAttendanceDateByNameAndDate의 과정에서 크루에 대한 객체를 불러와 Controller가 알고 있게 되면 그 이후에는 crew.edit(date, ...) 와 같이 서칭하는 로직이 한 번 줄어들 수 있을 것 같은데...
또 이 방법은 캡슐화를 깨는 것 같기도 하구요. 그저 메시지를 통해 행동을 이끌어내야 하는데 내부 상태를 가져와 직접 오더를 내리는 꼴이니... 객체지향적이지 않은 것 같다는 생각도 들어요.

객체 지향적인 것을 중점으로 둔다면 지금과 같은 방식이 비효율적으로 한 번 확인한 후 로직을 진행할 때 다시 확인하더라도 더 나을까요? 아니면 제가 완전히 잘못 생각하고 있는 걸까요?

AttendanceStatus 같은 경우에 지각, 결석을 구분하는 기준 분을 다음과 같이 정의해두었어요.

    private final static int TARDY_THRESHOLD_MINUTES = 5;
    private final static int ABSENCE_THRESHOLD_MINUTES = 30;

이 친구는 enum 클래스이기 때문에 상수로 두기보다, 상태로써 지니게 해도 될 것 같긴 해요. 다만 ATTENDANCE, TARDY, ABSENCE, NONE으로 정의를 하여 각 ATTENDANCE, NONE은 지니고 있을만한 값이 애매한 것 같아요. 0을 지니자니 너무 의미가 없는 값을 주어준 것 같구요 (like null?)... 어떤 방식이 더 좋은 방식일까요?

모델을 그대로 View의 파라미터로 넘기는 방법과 모델의 상태를 하나 하나 컨트롤러를 통해 View로 넘겨주는 방법 중 어떤 게 나은 방법일까요? 예를 들면... 컨트롤러에서 뷰로 model.getA(), model.getB()를 해주는 방법과, 컨트롤러에서는 뷰로 model을 넘겨주고 뷰 내부에서 model.getA(), model.getB()로 사용하는 방법,... 각 방법에서 차이점이 존재할지 궁금해요.

STEP1 때보다는 더 나은 설계, 코드, 질문들을 가지고 오고 싶어서 나름 신경을 써보았는데 여전히 부족하네요 ㅜ ㅜ
피드백 주신다면 여러 방면에서 잘 생각해보고 열심히 한 번 반영해보도록 하겠습니다 😸

연휴 마저 잘 보내세요 !!! 감사합니다 ㅎ

- 가독성을 위해 LocalDateTime을 파라미터로 받아오는 함수와, LocalDate, LocalTime을 받아오는 함수를 구분해주었습니다.

- 크루 전체의 출석 기록을 관리하는 클래스를 추가하였습니다. - AttendanceRecordTest 내부에 있던 검증단 테스트를 AttendanceBookTest에서 수행하도록 수정하였습니다.

- 해당 달의 시작일부터 오늘까지의 날짜가 아니라면 예외가 발생하도록 기능을 추가하였습니다.

- 출석 기록이 없는 결석의 경우에도 결석으로 표시될 수 있도록 수정하였습니다.

JunHoPark93

안녕하세요 시소~
2단계 구현하느라 고생많으셨습니다.
몇가지 피드백 남겼는데 확인 후 의견 및 반영부탁드릴게요.
도메인 구현위주로 피드백 반영해주시면 좋을거같아요~

JunHoPark93 · 2025-03-03T13:05:54Z

src/test/java/domain/AttendanceBookTest.java

@@ -0,0 +1,225 @@
+package domain;


지난 스텝 1 당시 TDD 사이클을 진행하다가도 어느 순간 선 테스트 작성을 잊고 기능 작성을 하여 테스트를 놓친 메서드가 존재했어요. 이를 방지하기 위해 TDD 사이클을 의식적으로 진행하여 테스트를 빠뜨리지 않도록 이전보다 신경써보려 노력했어요.
다만 이번에 이와 관련해서 진행 중 헷갈리는 부분이 있었어요.
일단 TDD 사이클을 실패 테스트 작성 > 성공하도록 비즈니스 로직 작성 > 성공 후 리팩터링 과정으로 진행했어요.
실패 테스트에 대한 기능을 크게 구현하고 이를 리펙터링하여 여러 작은 기능으로 분리하였는데, 이 때 메서드를 총 몇 가지로 분리할 것이라고 계획을 잡고 세부 메서드들에 대한 테스트를 먼저 작성 후 메서드를 분리하는 것이 맞는 순서인지, 아니면 메서드를 일단 분리한 후 각 메서드에 대한 테스트를 추가로 작성하는 것이 맞는 순서인지 헷갈려요! (분리 계획 잡고 계획에 맞게 테스트 작성 후 메서드 분리 vs 메서드 분리 후 각 분리된 메서드에 대한 테스트)

세부 메서드들이라함은 private메서드로 분리하는것으로 이해했는데 맞을까요?
말씀주신 vs에서 전자가 맞아보이는데요, 메서드가 지저분하게 만들어져도 성공하는 테스트를 만들어두었으면 자신있게 메서드를 리팩터링하거나 분리하거나 할 수 있으니까요.

이 부분을 의도하신게 아니라면 다시 말씀주세요!

이 부분을 의도한 게 맞습니다!! 답이 되었습니다 ㅎ

JunHoPark93 · 2025-03-03T13:09:18Z

src/main/java/domain/AttendanceBook.java

+
+    public void validateHasCrew(String name) {
+        if (!hasCrew(name)) {
+            throw new IllegalArgumentException("등록되지 않은 닉네임입니다.");


에러 메시지를 어떻게 관리해야 할까요? 저는 전체적인 예외 처리를 도메인에서 진행하도록 구현했어요. 그런데 진행하다보니 문제점이 있더라구요. View의 요구사항 변경으로 에러 메시지 내용만 달라지게 되면 Domain 코드에 변경이 일어나요. 예를 들면 12월 4일 토요일은 등교일이 아닙니다. 와 같은 경우에 출력 형식이 변경되면 말이죠. 이렇게 바라보니 예외 메시지는 View에서 부분인 것 같은데, Constants나 Enum 클래스로 따로 관리하고 패키지를 global 내부에 두어야 할지... 😿

이런 부분들 말씀이신거같네요!
사실 화면에 보여지는 디테일한 메세지의 책임은 화면이 가져가는게 맞는데, 현재는 자바 콘솔 애플리케이션이기때문에 발생하는 어려움인거같네요. 물론 하려면 할 수는 있지만, (최소정보만을 throw 하는 exception을 만들고 상세내역은 view에서 catch해서 처리) 지금 굳이 고려안하셔도 될거같습니다.
말씀주신대로 view요구사항에 도메인코드가 영향받는건 맞는데 이건 현재 미션에 종속된 특이사항으로 간주하고 넘어가도 되지 않을까 싶네요~

그렇군요 ㅎ ㅎ 아무래도 도메인 내부에서 에러 메시지를 관리하다보니 Custom 예외를 발생시켜야 할까? 등 여러 생각을 하게 되었는데, 이번 미션들에서는 그렇게 생각하고 진행해보겠습니다!!

JunHoPark93 · 2025-03-03T13:16:06Z

src/main/java/controller/AttendanceController.java

+        AttendanceDate originalAttendanceDate = attendanceBook.findAttendanceDateByNameAndDate(name, date);
+        attendanceBook.edit(name, date, enterAttendanceTimeForEdit());


다음과 같은 코드를 보게 되면 findAttendanceDateByNameAndDate를 통해 해당 크루에 대한 기록이 존재하는지 확인하구요,
edit도 이름을 통해 크루에 대한 출석 기록 수정이 이루어지는데, 그 내부는 모두 이름을 통해 크루가 있는지 확인을 한단 말이죠?
그러면 기존 findAttendanceDateByNameAndDate의 과정에서 크루에 대한 객체를 불러와 Controller가 알고 있게 되면 그 이후에는 crew.edit(date, ...) 와 같이 서칭하는 로직이 한 번 줄어들 수 있을 것 같은데...
또 이 방법은 캡슐화를 깨는 것 같기도 하구요. 그저 메시지를 통해 행동을 이끌어내야 하는데 내부 상태를 가져와 직접 오더를 내리는 꼴이니... 객체지향적이지 않은 것 같다는 생각도 들어요.

객체 지향적인 것을 중점으로 둔다면 지금과 같은 방식이 비효율적으로 한 번 확인한 후 로직을 진행할 때 다시 확인하더라도 더 나을까요? 아니면 제가 완전히 잘못 생각하고 있는 걸까요?

"crew.edit(date, ...) 와 같이 서칭하는 로직이 한 번 줄어들 수 있을 것 같은데..." 라고 말씀주신 부분은 edit안에서 find를 해주는것으로보이는데요, 이부분은 어색하지 않습니다. 찾아서 없으면 예외를 던지든 다른 처리를 하든 뭔가가 필요하니까요. 밖에서 original을 찾는 이유는 print하기 위함으로 이해했는데 맞을까요?
find를 최대한 한번 하는게 좋겠죠. 다만 구조상 그게 어려워서 2번을 하신거같은데 이 질문이 아니라면 다시 말씀부탁드려요~

이해하신 부분이 맞습니다!

아무래도 직전에 다른 작업에서 있는 게 확인이 되더라도 다른 작업시에는 다시 있는 걸 확인하는 게 옳은 구조인 것 같아요.
당시에는 순간적으로 어... 확인이 되었는데도 굳이 다시 확인을 해야할까? 싶었는데 그거는 프로그램 순서에 따라 발생한 결론이었던 것 같아요...

JunHoPark93 · 2025-03-03T13:20:14Z

src/main/java/domain/AttendanceStatus.java

-
-    public static AttendanceStatus attend(LocalDateTime target) {
-        LocalDate targetDate = target.toLocalDate();
+    ABSENCE,


AttendanceStatus 같은 경우에 지각, 결석을 구분하는 기준 분을 다음과 같이 정의해두었어요.
private final static int TARDY_THRESHOLD_MINUTES = 5;
private final static int ABSENCE_THRESHOLD_MINUTES = 30;
이 친구는 enum 클래스이기 때문에 상수로 두기보다, 상태로써 지니게 해도 될 것 같긴 해요. 다만 ATTENDANCE, TARDY, ABSENCE, NONE으로 정의를 하여 각 ATTENDANCE, NONE은 지니고 있을만한 값이 애매한 것 같아요. 0을 지니자니 너무 의미가 없는 값을 주어준 것 같구요 (like null?)... 어떤 방식이 더 좋은 방식일까요?

ATTENDANCE(0),
TARDY(5),
ABSENCE(30),
NONE(-1);
이런식으로 사용하시는걸 의미하신걸까요? 의미가 모호해지기때문에 현재 구조가 더 나아보입니다.

그렇군요! 감사합니다~~

JunHoPark93 · 2025-03-03T13:22:22Z

src/main/java/view/utils/ViewUtil.java

+        return "--:--";
+    }
+
+    public static String getAttendanceStatusMessage(AttendanceStatus attendanceStatus) {


모델을 그대로 View의 파라미터로 넘기는 방법과 모델의 상태를 하나 하나 컨트롤러를 통해 View로 넘겨주는 방법 중 어떤 게 나은 방법일까요? 예를 들면... 컨트롤러에서 뷰로 model.getA(), model.getB()를 해주는 방법과, 컨트롤러에서는 뷰로 model을 넘겨주고 뷰 내부에서 model.getA(), model.getB()로 사용하는 방법,... 각 방법에서 차이점이 존재할지 궁금해요.

현재 자바 콘솔애플리케이션에서는 차이가 없습니다. 지금은 통째로 넘겨주셔도 무방합니다~ 디테일하게 필요한 부분만 넘기는 과정은 추후 미션들에서 고려해주셔도 될것같습니다.

아하! 전달하는 방식에 대한 고민이 있었는데, 그렇게 하도록 하겠습니다 ㅎㅎ

JunHoPark93 · 2025-03-03T13:23:04Z

src/main/java/domain/AttendanceRecord.java

+        while (targetDate.isBefore(getFixedRunningDate())) {
+            if (isWeekday(targetDate)) {


depth를 줄일 수 잇는 방법이 있을지 고민해보면 어떨까요?

구현 도중에 출석하지 않은 날을 결석으로 판별하는 로직을 기존 생각한 설계와 다르게 구현하다보니 이 부분을 리펙터링하지 못했네요 ㅜ ㅜ

반영하였습니다!

JunHoPark93 · 2025-03-03T13:23:30Z

src/main/java/domain/AttendanceRecord.java

+        return attendanceDates.stream()
+                .filter(attendanceDate -> attendanceDate.isSameDate(date))
+                .findFirst()
+                .orElse(null);


null을 반환하게 되면 어떤 문제가 발생할 수 있을까요?

먼저 예상하지 못한 부분에서 NullPointerException이 발생할 수 있을 것 같아요.
저는 당시에 나름 null 관련 예외 발생 가능성이 없다고 보았지만, 예외는 말 그대로 예외니까요...

그리고 null이 나타내는 값에 대한 의미가 모호한 부분이 있을 것 같아요.
이 null이 객체가 빈 값이다? 그게 왜 비었지? 출석이 없기에 결석인 부분? 아니면 그저 결석? 여러 가지로 직관적이지도 못한 것 같구요.

그래서 이번 피드백 이후 Match 되는 Date가 있는지 확인하는 로직을 추가하여 조회가 필요한 경우 미리 검증하고, 조회 시 없다면 null 반환이 아닌 예외를 발생히는 로직으로 수정하여 해결해보고자 하였습니다!

JunHoPark93 · 2025-03-03T13:24:48Z

src/main/java/domain/AttendanceDate.java

+import java.time.LocalTime;
+
+public class AttendanceDate implements Comparable<AttendanceDate> {
+    private LocalDate date;


Suggested change

private LocalDate date;

private final LocalDate date;

이걸 빠뜨렸군요... 감사합니다 😿 😿

JunHoPark93 · 2025-03-03T13:25:38Z

src/main/java/domain/AttendanceDate.java

+    public AttendanceDate(LocalDateTime dateTime) {
+        this.date = dateTime.toLocalDate();
+        this.time = dateTime.toLocalTime();
+        saveAttendanceStatus();


이 부분은 굳이 메서드로 안빼도 될것같은데요, 오히려 date와 time을 나눠서 관리해야되는 이유가 있을지 고민해보시면 좋을것같습니다.

이 부분을 나눠서 관리하는 것은 사실 출석을 하지 않았기에 결석으로 보는 경우를 파악하기 위해서 도입을 했는데요,
아무래도 LocalDateTime.of(2024, 12, 10, 0, 0, 0)과 같은 느낌보다, LocalDate(2024, 12, 10), LocalTime(0, 0, 0)과 같이 값을 구분하여 사용하는 것이 시간이 비어있다 = 즉, 시간에만 특정 값을 쥐여주어 출석을 하지 않아 결석이다 라는 의미를 부여하기 위함이었습니다.

또... LocalDate와 LocalTime을 따로 관리하여 출석 날짜, 출석 시간이라는 값들을 명시적으로 구분하기 위함도 있었습니다.
아무래도 출석일시라는 값으로 관리하는 것보다 출석 날짜, 출석 시간이 구분되는 것이 의미적으로 명확하다는 제 개인적 견해가 있었습니다.

사실 이 메서드.. 말씀하신 것처럼 안 빼도 되긴 합니다만, 제가 만든 구조에서 LocalDateTime을 사용하려면 여러 군데에서 toLocalDate, toLocalTime을 호출하다보니 코드를 조금 더 깔끔하게 보기 위해 여러 생성자를 만들어 사용한 거라 ㅜ ㅜ 이런 불편함이 있긴 하네요.

혹시 이렇게 나눠서 관리하면 좋지 않은 이유가 있다면 알려주실 수 있을까요? 고민을 해보았지만, 제 지식으로는 굳이 나누면 안 되는 이유를 찾지 못했어요 ㅜ ㅜ 😿

JunHoPark93 · 2025-03-03T13:29:27Z

src/main/java/domain/AttendanceBook.java

+public class AttendanceBook {
+    private final Map<String, AttendanceRecord> attendance;


실제 사용자에 대한 도메인은 정의가 되어있지 않은데요, 해당 도메인을 정의해보는게 어떨까요?
현재는 단순히 map으로만 관리되고있는데 사용자와 사용자 그룹을 표현하는 도메인을 만들어보면 AttendanceBook의 역할과 크기를 줄여볼 수 있을것같습니다.

당시에 사용자에 대한 도메인은 관리할 상태가 단순히 이름만 있다고 판단했기 때문에 고려는 해보았지만 적용하지 않았어요.
그런데 피드백을 보고 난 후 ,사용자의 그룹을 만들어 사용자 자체를 관리할 필요가 있기 때문에 단순히 String으로 두기보다 객체로써 정의하고 AttendanceBook의 책임을 줄일 수 있도록 그룹 객체를 만드는 게 훨씬 좋은 구조인 것 같아요.

그렇게 반영하였습니다!

egaeng09

개인적인 일정이 조금 있었어서 이번 미션 피드백 반영이 조금 늦었네요... 빠르게 하고 싶었는데 ㅜ ㅜ 바쁘실텐데 죄송합니다 😿

피드백 주신 부분들 곰곰이 생각해보고 반영해보았습니다.

감사합니다!!

egaeng09 · 2025-03-05T12:18:19Z

src/main/java/controller/AttendanceController.java

+        AttendanceDate originalAttendanceDate = attendanceBook.findAttendanceDateByNameAndDate(name, date);
+        attendanceBook.edit(name, date, enterAttendanceTimeForEdit());


이해하신 부분이 맞습니다!

아무래도 직전에 다른 작업에서 있는 게 확인이 되더라도 다른 작업시에는 다시 있는 걸 확인하는 게 옳은 구조인 것 같아요.
당시에는 순간적으로 어... 확인이 되었는데도 굳이 다시 확인을 해야할까? 싶었는데 그거는 프로그램 순서에 따라 발생한 결론이었던 것 같아요...

egaeng09 · 2025-03-05T12:19:50Z

src/main/java/domain/AttendanceBook.java

+public class AttendanceBook {
+    private final Map<String, AttendanceRecord> attendance;


당시에 사용자에 대한 도메인은 관리할 상태가 단순히 이름만 있다고 판단했기 때문에 고려는 해보았지만 적용하지 않았어요.
그런데 피드백을 보고 난 후 ,사용자의 그룹을 만들어 사용자 자체를 관리할 필요가 있기 때문에 단순히 String으로 두기보다 객체로써 정의하고 AttendanceBook의 책임을 줄일 수 있도록 그룹 객체를 만드는 게 훨씬 좋은 구조인 것 같아요.

그렇게 반영하였습니다!

egaeng09 · 2025-03-05T12:20:48Z

src/main/java/domain/AttendanceBook.java

+
+    public void validateHasCrew(String name) {
+        if (!hasCrew(name)) {
+            throw new IllegalArgumentException("등록되지 않은 닉네임입니다.");


그렇군요 ㅎ ㅎ 아무래도 도메인 내부에서 에러 메시지를 관리하다보니 Custom 예외를 발생시켜야 할까? 등 여러 생각을 하게 되었는데, 이번 미션들에서는 그렇게 생각하고 진행해보겠습니다!!

egaeng09 · 2025-03-05T12:27:12Z

src/main/java/domain/AttendanceDate.java

+    public AttendanceDate(LocalDateTime dateTime) {
+        this.date = dateTime.toLocalDate();
+        this.time = dateTime.toLocalTime();
+        saveAttendanceStatus();


이 부분을 나눠서 관리하는 것은 사실 출석을 하지 않았기에 결석으로 보는 경우를 파악하기 위해서 도입을 했는데요,
아무래도 LocalDateTime.of(2024, 12, 10, 0, 0, 0)과 같은 느낌보다, LocalDate(2024, 12, 10), LocalTime(0, 0, 0)과 같이 값을 구분하여 사용하는 것이 시간이 비어있다 = 즉, 시간에만 특정 값을 쥐여주어 출석을 하지 않아 결석이다 라는 의미를 부여하기 위함이었습니다.

또... LocalDate와 LocalTime을 따로 관리하여 출석 날짜, 출석 시간이라는 값들을 명시적으로 구분하기 위함도 있었습니다.
아무래도 출석일시라는 값으로 관리하는 것보다 출석 날짜, 출석 시간이 구분되는 것이 의미적으로 명확하다는 제 개인적 견해가 있었습니다.

사실 이 메서드.. 말씀하신 것처럼 안 빼도 되긴 합니다만, 제가 만든 구조에서 LocalDateTime을 사용하려면 여러 군데에서 toLocalDate, toLocalTime을 호출하다보니 코드를 조금 더 깔끔하게 보기 위해 여러 생성자를 만들어 사용한 거라 ㅜ ㅜ 이런 불편함이 있긴 하네요.

혹시 이렇게 나눠서 관리하면 좋지 않은 이유가 있다면 알려주실 수 있을까요? 고민을 해보았지만, 제 지식으로는 굳이 나누면 안 되는 이유를 찾지 못했어요 ㅜ ㅜ 😿

egaeng09 · 2025-03-05T12:32:49Z

src/main/java/domain/AttendanceRecord.java

+        return attendanceDates.stream()
+                .filter(attendanceDate -> attendanceDate.isSameDate(date))
+                .findFirst()
+                .orElse(null);


먼저 예상하지 못한 부분에서 NullPointerException이 발생할 수 있을 것 같아요.
저는 당시에 나름 null 관련 예외 발생 가능성이 없다고 보았지만, 예외는 말 그대로 예외니까요...

그리고 null이 나타내는 값에 대한 의미가 모호한 부분이 있을 것 같아요.
이 null이 객체가 빈 값이다? 그게 왜 비었지? 출석이 없기에 결석인 부분? 아니면 그저 결석? 여러 가지로 직관적이지도 못한 것 같구요.

그래서 이번 피드백 이후 Match 되는 Date가 있는지 확인하는 로직을 추가하여 조회가 필요한 경우 미리 검증하고, 조회 시 없다면 null 반환이 아닌 예외를 발생히는 로직으로 수정하여 해결해보고자 하였습니다!

egaeng09 · 2025-03-05T12:32:57Z

src/main/java/domain/AttendanceStatus.java

-
-    public static AttendanceStatus attend(LocalDateTime target) {
-        LocalDate targetDate = target.toLocalDate();
+    ABSENCE,


그렇군요! 감사합니다~~

egaeng09 · 2025-03-05T12:33:30Z

src/main/java/view/utils/ViewUtil.java

+        return "--:--";
+    }
+
+    public static String getAttendanceStatusMessage(AttendanceStatus attendanceStatus) {


아하! 전달하는 방식에 대한 고민이 있었는데, 그렇게 하도록 하겠습니다 ㅎㅎ

egaeng09 · 2025-03-05T12:34:08Z

src/test/java/domain/AttendanceBookTest.java

@@ -0,0 +1,225 @@
+package domain;


이 부분을 의도한 게 맞습니다!! 답이 되었습니다 ㅎ

egaeng09 · 2025-03-05T15:04:30Z

src/main/java/domain/AttendanceRecord.java

+        while (targetDate.isBefore(getFixedRunningDate())) {
+            if (isWeekday(targetDate)) {


구현 도중에 출석하지 않은 날을 결석으로 판별하는 로직을 기존 생각한 설계와 다르게 구현하다보니 이 부분을 리펙터링하지 못했네요 ㅜ ㅜ

반영하였습니다!

egaeng09 · 2025-03-05T15:23:46Z

src/main/java/domain/AttendanceDate.java

+import java.time.LocalTime;
+
+public class AttendanceDate implements Comparable<AttendanceDate> {
+    private LocalDate date;


이걸 빠뜨렸군요... 감사합니다 😿 😿

egaeng09 added 30 commits February 27, 2025 14:48

chore: 2단계 진행을 위해 파일 전체 삭제

c70598b

docs: 구현 기능 목록 작성

143c122

feat: 교육 시간에 따른 출석 판별 기능 추가

c73a794

feat: 출석 일자, 시간 관리 클래스 추가

0d46f22

refactor: 일부 변수명 카멜 케이스로 수정

92b9c45

feat: 출석 기록 관리 클래스 추가

61016d5

refactor: 출석 기록 추가하는 메서드 오버로딩

f6f8064

- 가독성을 위해 LocalDateTime을 파라미터로 받아오는 함수와, LocalDate, LocalTime을 받아오는 함수를 구분해주었습니다.

feat: 전체 출석 기록 관리 클래스 추가

269890b

- 크루 전체의 출석 기록을 관리하는 클래스를 추가하였습니다. - AttendanceRecordTest 내부에 있던 검증단 테스트를 AttendanceBookTest에서 수행하도록 수정하였습니다.

fix: 출석 수정 테스트 조건 수정

8bbfab0

feat: 캠퍼스 운영 시간, 등교일 검증 기능 추가

301bcf0

feat: 제적 위험도 판단 기능 추가

bf54b97

feat: 지각, 결석 횟수 계산 기능 추가

49bdf81

docs: 기능 목록 추가

5b74e01

refactor: 불필요한 메서드 제거

4feccdd

feat: 출석 가능 일자 판별 기능 추가

1289818

- 해당 달의 시작일부터 오늘까지의 날짜가 아니라면 예외가 발생하도록 기능을 추가하였습니다.

feat: 출석 수정 로직 변경

c8ebbb4

refactor: 패키지 이동 및 날짜 관련 유틸 함수 추가

fec09f6

refactor: 초기 출석 정보 입력 함수 네이밍 변경

3eaf99a

feat: 출석 가능한 날짜 검증 로직 추가

c9cf42b

feat: 날짜 형식 및 출력 형식 변환 기능 추가

ff49a76

feat: 미출석 체크를 위해 데이터 초기화 기능 추가 및 출석, 수정 로직 변경

444a5db

feat: View, Controller 추가 및 메뉴 선택 기능 구현

febf11a

feat: 출석 횟수 계산 로직 추가

6049e22

refactor: 출석 상태 출력 메시지 로직 수정

b8fd736

- 출석 기록이 없는 결석의 경우에도 결석으로 표시될 수 있도록 수정하였습니다.

feat: 출석 기능 구현

e462d14

feat: 출석 수정 및 크루별 출석 기록 확인 기능 구현

180dac9

feat: 크루별 출석 기록 확인 및 제적 위험자 확인 기능 구현

2f67223

feat: 출석, 수정 전 검증 로직 추가

aa9cb40

feat: 파일 입력 기능 추가

75b8622

refactor: 컨벤션 수정 및 미사용 메서드 제거

7f07f3d

egaeng09 changed the base branch from main to egaeng09 March 2, 2025 20:14

JunHoPark93 requested changes Mar 3, 2025

View reviewed changes

egaeng09 added 4 commits March 4, 2025 11:48

refactor: 컨벤션 수정

1938821

feat: 제적 위험자 조회 정렬 기능 추가

bcd7c97

refactor: Crew 클래스 추가 및 Crew 관리 책임을 Crews 클래스로 위임

81b122b

refactor: 메서드 depth 축소 및 null 처리 로직 수정

a489261

egaeng09 commented Mar 5, 2025

View reviewed changes

refactor: 날짜 상태 final 키워드 추가

c7c9dfb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 출석 다시 구현하기] 시소(이경민) 미션 제출합니다. #149

[2단계 - 출석 다시 구현하기] 시소(이경민) 미션 제출합니다. #149

egaeng09 commented Mar 2, 2025 •

edited

Loading

JunHoPark93 left a comment

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025

JunHoPark93 Mar 3, 2025

egaeng09 Mar 5, 2025 •

edited

Loading

egaeng09 left a comment

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025 •

edited

Loading

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

egaeng09 Mar 5, 2025

		AttendanceDate originalAttendanceDate = attendanceBook.findAttendanceDateByNameAndDate(name, date);
		attendanceBook.edit(name, date, enterAttendanceTimeForEdit());

		while (targetDate.isBefore(getFixedRunningDate())) {
		if (isWeekday(targetDate)) {

		public class AttendanceBook {
		private final Map<String, AttendanceRecord> attendance;

[2단계 - 출석 다시 구현하기] 시소(이경민) 미션 제출합니다. #149

Are you sure you want to change the base?

[2단계 - 출석 다시 구현하기] 시소(이경민) 미션 제출합니다. #149

Conversation

egaeng09 commented Mar 2, 2025 • edited Loading

체크 리스트

객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?

1~5점 중에서 선택해주세요.

선택한 점수의 이유를 적어주세요.

어떤 부분에 집중하여 리뷰해야 할까요?

JunHoPark93 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

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

egaeng09 Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

egaeng09 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egaeng09 Mar 5, 2025 • edited Loading

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

egaeng09 commented Mar 2, 2025 •

edited

Loading

egaeng09 Mar 5, 2025 •

edited

Loading

egaeng09 Mar 5, 2025 •

edited

Loading