[REFACTOR] 타임박스 도메인 - 엔티티 분리 / 서비스 로직 개선#201
Conversation
- domain.customize -> entity.customize 패키지로 변경
|
""" Walkthrough타임박스 도메인과 엔티티를 분리하는 리팩터링이 진행되었습니다. 기존 도메인 계층의 Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant CustomizeServiceV2
participant CustomizeTimeBoxRepository
participant BellRepository
Controller->>CustomizeServiceV2: updateTable(request, tableId, member)
CustomizeServiceV2->>BellRepository: deleteAllByTable(tableId)
CustomizeServiceV2->>CustomizeTimeBoxRepository: deleteAllByTable(tableId)
CustomizeServiceV2->>CustomizeTimeBoxRepository: saveAll(timeBoxEntities)
CustomizeServiceV2->>BellRepository: saveAll(bellEntities)
CustomizeServiceV2-->>Controller: return CustomizeTableResponse
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 사항 없음) Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Test Results 85 files 85 suites 12s ⏱️ Results for commit 6917a44. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
40-45: getBells 메서드의 성능을 개선할 수 있습니다.현재 구현은 각 타임박스마다 모든 벨을 순회하여 필터링합니다 (O(n*m) 복잡도). 벨이 많은 경우 성능 문제가 발생할 수 있습니다.
다음과 같이 Map을 사용하여 성능을 개선할 수 있습니다:
public class CustomizeTimeBoxEntities { private static final Comparator<CustomizeTimeBoxEntity> TIME_BOX_COMPARATOR = Comparator .comparing(CustomizeTimeBoxEntity::getSequence); @Getter private final List<CustomizeTimeBoxEntity> timeBoxes; private final List<BellEntity> bells; + private final Map<CustomizeTimeBoxEntity, List<Bell>> bellsByTimeBox; public CustomizeTimeBoxEntities(List<CustomizeTimeBoxEntity> timeBoxes) { this.timeBoxes = timeBoxes.stream() .sorted(TIME_BOX_COMPARATOR) .toList(); this.bells = Collections.emptyList(); + this.bellsByTimeBox = Collections.emptyMap(); } public CustomizeTimeBoxEntities(List<CustomizeTimeBoxEntity> timeBoxes, List<BellEntity> bells) { this.timeBoxes = timeBoxes.stream() .sorted(TIME_BOX_COMPARATOR) .toList(); this.bells = bells; + this.bellsByTimeBox = bells.stream() + .collect(Collectors.groupingBy( + bell -> timeBoxes.stream() + .filter(bell::isContained) + .findFirst() + .orElse(null), + Collectors.mapping(BellEntity::toDomain, Collectors.toList()) + )); } public List<CustomizeTimeBox> toDomain() { return timeBoxes.stream() - .map(timebox -> timebox.toDomain(getBells(timebox))) + .map(timebox -> timebox.toDomain(bellsByTimeBox.getOrDefault(timebox, Collections.emptyList()))) .toList(); } - - private List<Bell> getBells(CustomizeTimeBoxEntity timeBox) { - return bells.stream() - .filter(bell -> bell.isContained(timeBox)) - .map(BellEntity::toDomain) - .toList(); - } }src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntity.java (1)
118-128: 도메인 객체로부터 엔티티를 생성하는 생성자에서 유효성 검증을 고려해보세요.기존 생성자들과 달리 이 생성자는 유효성 검증 로직이 없습니다. 도메인 객체에서 이미 검증이 완료되었다고 가정하는 것 같지만, 엔티티 레벨에서도 데이터 무결성을 보장하는 것이 좋겠습니다.
특히
time필드가 nullable로 변경되었으므로, 적절한 null 처리 검증을 추가하는 것을 고려해보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/debatetimer/domain/customize/CustomizeTimeBoxEntities.java(0 hunks)src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java(1 hunks)src/main/java/com/debatetimer/dto/customize/response/CustomizeTableResponse.java(1 hunks)src/main/java/com/debatetimer/entity/customize/BellEntity.java(3 hunks)src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java(1 hunks)src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntity.java(5 hunks)src/main/java/com/debatetimer/repository/customize/BellRepository.java(1 hunks)src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.java(2 hunks)src/main/java/com/debatetimer/service/customize/CustomizeService.java(3 hunks)src/main/java/com/debatetimer/service/customize/CustomizeServiceV2.java(3 hunks)src/test/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntitiesTest.java(1 hunks)src/test/java/com/debatetimer/repository/BaseRepositoryTest.java(3 hunks)src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java(1 hunks)src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java(3 hunks)src/test/java/com/debatetimer/service/customize/CustomizeServiceV2Test.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/debatetimer/domain/customize/CustomizeTimeBoxEntities.java
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
src/test/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntitiesTest.java (2)
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntity.java (2)
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java (2)
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.847Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
🧬 Code Graph Analysis (6)
src/main/java/com/debatetimer/dto/customize/response/CustomizeTableResponse.java (1)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
CustomizeTimeBoxEntities(10-46)
src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.java (1)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
CustomizeTimeBoxEntities(10-46)
src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java (2)
src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java (2)
Nested(21-39)Nested(41-80)src/test/java/com/debatetimer/service/customize/CustomizeServiceV2Test.java (5)
Nested(33-62)Nested(64-98)Nested(100-174)Nested(176-205)Nested(207-242)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntity.java (3)
src/main/java/com/debatetimer/domain/customize/CustomizeTimeBox.java (1)
CustomizeTimeBox(8-75)src/main/java/com/debatetimer/domain/customize/NormalTimeBox.java (1)
NormalTimeBox(8-57)src/main/java/com/debatetimer/domain/customize/TimeBasedTimeBox.java (1)
TimeBasedTimeBox(9-79)
src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java (1)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
CustomizeTimeBoxEntities(10-46)
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
src/main/java/com/debatetimer/domain/customize/CustomizeTimeBox.java (1)
CustomizeTimeBox(8-75)
🔇 Additional comments (24)
src/main/java/com/debatetimer/service/customize/CustomizeService.java (3)
8-8: 패키지 변경이 리팩터링과 일치합니다.
CustomizeTimeBoxEntities클래스가 도메인 패키지에서 엔티티 패키지로 이동한 것과 일치하는 변경사항입니다.
50-50: 레포지토리 메서드 시그니처 변경에 맞춰 개선되었습니다.엔티티 객체 전체를 전달하는 대신 테이블 ID만 전달하도록 변경되어 더 효율적이고 명확해졌습니다.
67-67: 일관성 있는 변경사항입니다.
updateTable메서드와 동일한 패턴으로 테이블 ID를 전달하도록 변경되어 코드의 일관성이 유지되었습니다.src/test/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntitiesTest.java (2)
1-1: 패키지 이동이 올바르게 반영되었습니다.
CustomizeTimeBoxEntities클래스가 엔티티 패키지로 이동한 것에 맞춰 테스트 클래스의 패키지도 적절히 변경되었습니다.
5-7: 임포트 구문이 새로운 패키지 구조에 맞게 업데이트되었습니다.도메인 클래스들을 명시적으로 임포트하고 같은 패키지의 엔티티 클래스들은 제거하여 깔끔한 구조를 유지했습니다.
src/test/java/com/debatetimer/repository/BaseRepositoryTest.java (3)
4-4: 테스트 픽스처 추가가 적절합니다.새로운
BellRepository테스트를 지원하기 위한BellGenerator임포트가 추가되었습니다.
16-17: Spring 컨텍스트 설정이 올바르게 업데이트되었습니다.
@Import어노테이션에BellGenerator가 추가되어 테스트에서 사용할 수 있도록 설정되었습니다.
31-32: 일관성 있는 테스트 픽스처 패턴입니다.다른 Generator들과 동일한 패턴으로
BellGenerator필드가 추가되어 하위 테스트 클래스에서 사용할 수 있습니다.src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.java (2)
4-4: 패키지 변경이 리팩터링과 일치합니다.
CustomizeTimeBoxEntities가 엔티티 패키지로 이동한 것에 맞춰 임포트가 업데이트되었습니다.
30-32: 메서드 시그니처 개선이 효율적입니다.엔티티 객체 전체 대신 테이블 ID만 받도록 변경되어 더 효율적이고 명확한 인터페이스가 되었습니다. JPQL 쿼리도 적절히 수정되었습니다.
src/main/java/com/debatetimer/dto/customize/response/CustomizeTableResponse.java (1)
5-5: 임포트 변경이 리팩터링과 일치합니다.
CustomizeTimeBoxEntities클래스가 엔티티 패키지로 이동한 것에 맞춰 임포트가 업데이트되었습니다. 기능적 변경은 없고 패키지 구조 변경만 반영된 적절한 변경사항입니다.src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java (1)
5-5: 패키지 이동에 따른 import 문 수정이 적절합니다.
CustomizeTimeBoxEntities클래스가 domain 패키지에서 entity 패키지로 이동함에 따라 import 문이 올바르게 업데이트되었습니다. 이는 도메인과 엔티티 계층 분리라는 PR 목표와 일치합니다.src/test/java/com/debatetimer/service/customize/CustomizeServiceV2Test.java (1)
83-83: 서비스 계층 변경에 맞춘 테스트 수정이 적절합니다.벨이 없는 경우 빈 리스트 대신
null을 반환하도록 서비스 로직이 변경됨에 따라 테스트 assertion이 올바르게 업데이트되었습니다. 이는 도메인 엔티티 중심 설계 변경과 일치합니다.src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java (1)
51-51: 리포지토리 메서드 시그니처 변경에 맞춘 테스트 수정이 적절합니다.
deleteAllByTable메서드가 엔티티 객체 대신 테이블 ID를 받도록 변경됨에 따라 모든 테스트 호출이 올바르게 업데이트되었습니다. 이는 성능 향상과 함께 일관된 삭제 패턴을 제공합니다.Also applies to: 66-66, 77-77
src/main/java/com/debatetimer/entity/customize/BellEntity.java (3)
3-3: 도메인 객체 import 추가가 적절합니다.도메인-엔티티 변환 기능을 위한
Bell도메인 객체 import가 추가되었습니다.
68-70: 도메인 변환 메서드 구현이 적절합니다.엔티티를 도메인 객체로 변환하는
toDomain()메서드가 깔끔하게 구현되었습니다. 도메인-엔티티 분리 패턴에 잘 맞습니다.
72-74: ID 비교를 통한 포함 여부 확인 로직이 효율적입니다.타임박스와의 연관성을 ID 비교로 확인하는 방법이 효율적이며,
CustomizeTimeBoxEntities클래스에서 벨 필터링에 적절히 활용됩니다.src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java (1)
1-47: 새로운 벨 리포지토리 테스트 구현이 우수합니다.
deleteAllByTable메서드를 위한 테스트가 잘 구성되어 있습니다:
- 테이블 ID로 벨을 삭제하는 새로운 기능을 적절히 테스트
- 다른 테이블의 벨이 영향받지 않는지 확인하는 격리성 테스트
- 중첩 클래스와 한국어 메서드명으로 기존 패턴과 일관성 유지
assertAll을 사용한 포괄적인 검증src/main/java/com/debatetimer/repository/customize/BellRepository.java (1)
14-16: JPQL 쿼리와 @Modifying 어노테이션이 적절히 구성되었습니다.테이블 ID를 기반으로 벨 엔티티를 삭제하는 방식으로 변경한 것이 좋습니다. 연관 관계를 통한 삭제 쿼리가 명확하고,
clearAutomatically와flushAutomatically옵션으로 영속성 컨텍스트 관리도 적절합니다.src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntities.java (1)
10-33: 엔티티 래퍼 클래스가 잘 설계되었습니다.도메인과 엔티티를 분리하면서 변환 책임을 명확히 한 좋은 설계입니다. 정렬 로직도 적절히 구현되었습니다.
src/main/java/com/debatetimer/service/customize/CustomizeServiceV2.java (2)
87-97: 도메인 중심의 저장 로직이 깔끔하게 구현되었습니다.도메인 객체를 직접 사용하여 엔티티를 생성하고 저장하는 방식이 명확합니다. 시퀀스 관리도 적절합니다.
55-62: 외부 엔티티 ID 참조 문제 없음
- BellEntity, CustomizeTimeBoxEntity의 ID는 코드베이스 내 다른 모듈에서 참조되는 부분이 없습니다.
- 삭제 후 재생성 방식으로 인한 ID 변경이 현재 로직에 영향을 주지 않음을 확인했습니다.
[추가 고려]
- 감사(audit) 목적의 엔티티 변경 이력 관리가 필요하다면, 삭제·재생성 대신 변경 이력 테이블 도입이나 버전 관리 전략을 검토하세요.
src/main/java/com/debatetimer/entity/customize/CustomizeTimeBoxEntity.java (2)
3-8: 도메인 객체와의 변환을 위한 import 구문들이 적절히 추가되었습니다.도메인-엔티티 변환 로직을 위해 필요한 클래스들이 올바르게 import되었습니다.
Also applies to: 23-23
199-204: 엔티티를 도메인 객체로 변환하는 로직이 올바르게 구현되었습니다.
boxType에 따라 적절한 도메인 객체를 생성하고, bells는NormalTimeBox에만 전달하는 로직이 도메인 설계와 일치합니다.
- deleteAll() 요청 시, 영속성 컨텍스트가 플러시 된다는 것을 고려하여 순서 수정
| @Query("DELETE FROM BellEntity b WHERE b.customizeTimeBox.customizeTable.id = :tableId") | ||
| @Modifying(clearAutomatically = true, flushAutomatically = true) | ||
| void deleteAllByTable(long tableId); |
There was a problem hiding this comment.
이거 쿼리가 궁금해서 한번 테스트로 확인해봤더니
delete
from
bell dml_target_
where
exists(select
1
from
bell be1_0
join
customize_time_box ctb1_0
on ctb1_0.id=be1_0.customize_time_box_id
where
ctb1_0.table_id=?
and dml_target_._rowid_=be1_0._rowid_)
흠... 데이터 양을 생각하면 괜찮을 것 같긴한데... 나중에 최적화를 해보면 좋을 것 같네요
There was a problem hiding this comment.
[궁금한 질문]
이거 진짜 그냥 궁금해서 남기는 질문인데 timeBox in 절이 아니라 tableid 받아서 삭제하게 한 이유가 있나요? 사용성 측면에선 더 좋아졌다고 생각하는데 도메인 적으로는 시간표 - 타임박스의 벨이 약간 거리감이 있는 느낌이라 커찬이 생각한 리팩터링 의도가 순수히 궁금해요!
There was a problem hiding this comment.
동시성 이슈 기억하시나요?
timeBox 조회 -> 삭제 하는 로직 덕분에 동시성 이슈 일어났잖아요. deleteByTimeBoxIn(List<TimeBox> timBoxes) 이런 거 쓰면 timeBox를 조회해야 되니까 동시성 이슈 해결이 안되겠죠?
| public CustomizeTimeBoxEntities(List<CustomizeTimeBoxEntity> timeBoxes, List<BellEntity> bells) { | ||
| this.timeBoxes = timeBoxes.stream() | ||
| .sorted(TIME_BOX_COMPARATOR) | ||
| .toList(); | ||
| this.bells = bells; | ||
| } |
There was a problem hiding this comment.
나중에 V1 제거할 때 bell을 포함하는 이름으로 바꿔야 할려나요
There was a problem hiding this comment.
아 이게... TimeBox 안에 Bell 이 포함된 개념이라고 생각해서 bell 관련 정보를 따로 명시하지는 않았음(ex. Car 안에 Name이 포함되어 있는 느낌?)
물론, 인지 방향이 어느정도 떨어질 수 있다는 것에는 일부 동의하긴 합니다. 그래도 나는 개념을 명확히 하고 서로 공유하는 컨텍스트에 있다면 위와 같은 클래스 네임도 납득할 수 있다고 생각합니다.
There was a problem hiding this comment.
반영해서 사용하지 않는 생성자 삭제했습니다~ (CustomizeTableResponse, CustomizeTimeBoxResponse)
| private void saveTimeBoxes(CustomizeTableEntity tableEntity, List<CustomizeTimeBox> timeBoxes) { | ||
| IntStream.range(0, timeBoxes.size()) | ||
| .forEach(i -> saveTimeBox(tableEntity, timeBoxes.get(i), i + 1)); | ||
| } | ||
|
|
||
| private void deleteBell(CustomizeTimeBoxEntities savedCustomizeTimeBoxes) { | ||
| bellRepository.deleteAllByCustomizeTimeBoxIn(savedCustomizeTimeBoxes.getTimeBoxes()); | ||
| private void saveTimeBox(CustomizeTableEntity tableEntity, CustomizeTimeBox timeBox, int sequence) { | ||
| CustomizeTimeBoxEntity timeBoxEntity = timeBoxRepository.save( | ||
| new CustomizeTimeBoxEntity(tableEntity, timeBox, sequence)); | ||
| timeBox.getBells() | ||
| .forEach(bell -> bellRepository.save(new BellEntity(timeBoxEntity, bell))); |
There was a problem hiding this comment.
bell도 저장한다는 의미가 들어가는 네이밍은 어떤가요?
There was a problem hiding this comment.
위에서도 같이 언급했지만 TimeBox 안에 Bell 이 포함된 개념이라고 생각해서 bell 관련 정보를 따로 명시하지는 않았음
이거 관련해서는 디스코드에서 더 논의해보면 좋을 것 같아요!
There was a problem hiding this comment.
- 벨과 timeBox가 모든 로직에서 같이 응집되어 움직이면 커찬 의견에 동의
- 만약
-
- 박스와 별개로 종소리를 생성, 수정, 삭제하는 요청은 없으며, 없을 예정인가?
-
- 종소리와 별개로, 박스를 생성, 수정, 삭제하는 요청은 없으며, 없을 예정인가?
-
- 다만, timeBox에 대한 CRUD 요청에 모두 Bell이 내포되어 있다는 관념이 모든 팀원들에게 align 되면 좋을 듯!
- 지금으로선 없을 것 같은데 1번의 경우 추후에 어떻게 될지는 모르겠네요
- 사용하지 않는 생성자 제거 - 정팩매 -> 생성자로 변경
|
/noti 비토 리뷰에 대한 코멘트 달았습니다~ |
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
커찬! 어제 일정 소화 후 쓰러져가지고 오늘 일찍 일어나 리뷰 남겨요 😭
- timeBox 내에 bell이 포함되었을 것이라는 가정에 대한 팀원 간 의견 합치
이것만 회의에서 다루면 좋겠다는 생각이고
몇가지 의견 남겼으니 검토 부탁합니다!
| } | ||
|
|
||
| public boolean isContained(CustomizeTimeBoxEntity timeBox) { | ||
| return this.customizeTimeBox.getId().equals(timeBox.getId()); |
There was a problem hiding this comment.
[사소한 제안]
CustomizeTimeBoxEntity 한테 isSame(CustomizeTimeBoxEntity)로 시켜도 될 것 같아요
| private static final Comparator<CustomizeTimeBoxEntity> TIME_BOX_COMPARATOR = Comparator | ||
| .comparing(CustomizeTimeBoxEntity::getSequence); | ||
|
|
||
| @Getter |
| @Query("DELETE FROM BellEntity b WHERE b.customizeTimeBox.customizeTable.id = :tableId") | ||
| @Modifying(clearAutomatically = true, flushAutomatically = true) | ||
| void deleteAllByTable(long tableId); |
There was a problem hiding this comment.
[궁금한 질문]
이거 진짜 그냥 궁금해서 남기는 질문인데 timeBox in 절이 아니라 tableid 받아서 삭제하게 한 이유가 있나요? 사용성 측면에선 더 좋아졌다고 생각하는데 도메인 적으로는 시간표 - 타임박스의 벨이 약간 거리감이 있는 느낌이라 커찬이 생각한 리팩터링 의도가 순수히 궁금해요!
| private void saveTimeBoxes(CustomizeTableEntity tableEntity, List<CustomizeTimeBox> timeBoxes) { | ||
| IntStream.range(0, timeBoxes.size()) | ||
| .forEach(i -> saveTimeBox(tableEntity, timeBoxes.get(i), i + 1)); | ||
| } | ||
|
|
||
| private void deleteBell(CustomizeTimeBoxEntities savedCustomizeTimeBoxes) { | ||
| bellRepository.deleteAllByCustomizeTimeBoxIn(savedCustomizeTimeBoxes.getTimeBoxes()); | ||
| private void saveTimeBox(CustomizeTableEntity tableEntity, CustomizeTimeBox timeBox, int sequence) { | ||
| CustomizeTimeBoxEntity timeBoxEntity = timeBoxRepository.save( | ||
| new CustomizeTimeBoxEntity(tableEntity, timeBox, sequence)); | ||
| timeBox.getBells() | ||
| .forEach(bell -> bellRepository.save(new BellEntity(timeBoxEntity, bell))); |
There was a problem hiding this comment.
- 벨과 timeBox가 모든 로직에서 같이 응집되어 움직이면 커찬 의견에 동의
- 만약
-
- 박스와 별개로 종소리를 생성, 수정, 삭제하는 요청은 없으며, 없을 예정인가?
-
- 종소리와 별개로, 박스를 생성, 수정, 삭제하는 요청은 없으며, 없을 예정인가?
-
- 다만, timeBox에 대한 CRUD 요청에 모두 Bell이 내포되어 있다는 관념이 모든 팀원들에게 align 되면 좋을 듯!
- 지금으로선 없을 것 같은데 1번의 경우 추후에 어떻게 될지는 모르겠네요
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
회의에서 다 나눈 얘기이므로 approve 합니다.
/noti
|
/noti @debate-timer/backend
|
3 participants
🚩 연관 이슈
closed #191
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
테스트