[레거시 코드 리팩터링 - 1단계] 후디(조동현) 미션 제출합니다. #250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ulimy
suggested changes
Oct 25, 2022
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class DatabaseCleaner { |
There was a problem hiding this comment.
truncate.sql 등과 같이 데이터베이스를 초기화하는 sql문을 저장해놓고 실행시키거나, dao의 deleteAll() 메서드를 사용하는 방법도 있을텐데 해당 방법을 택하신 이유가 궁금해요!
There was a problem hiding this comment.
truncate.sql 혹은 DAO를 사용하는 방법 모두 좋은 방법인 것 같아요. 제가 DatabaseCleaner 를 직접 구현해서 사용한 이유는 테이블이 생기거나, 제거되어도 코드를 수정하지 않도록 하기 위함이었어요. truncate.sql 과 DAO를 사용한 방법은 테이블이 생기거나 제거될때마다 코드를 수정해야 한다는 점이 조금 불편하더라구요.
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| this.menuGroup = menuGroupDao.save( |
There was a problem hiding this comment.
this는 매개 변수와 객체 자신이 가지고 있는 변수의 이름이 같은 경우 이를 구분하기 위해 사용한다고 합니다. 이 경우에서는 필요없을 것 같아요!
| class CreateMethod { | ||
| @DisplayName("메뉴 그룹을 생성한다.") | ||
| @Test | ||
| void Should_CreateMenuGroup() { |
There was a problem hiding this comment.
찾아보니 이렇게 테스트 메소드를 작성하는 컨벤션이 있더라구요. 큰 의미는 없습니다.
| Menu actual = menuService.create(menu); | ||
|
|
||
| // then | ||
| assertAll(() -> { |
There was a problem hiding this comment.
개인적인 취향이지만 create 전후에서 가장 크게 달라지는 점은 id가 생성되는 것이라 생각해요. id가 비어있는지 확인하는 테스트코드가 있다면 어떨까요? 이 부분은 매우 개인적이니 반영하지 않으셔도 괜찮습니다!
There was a problem hiding this comment.
공감이 가네요! 엔티티 생성 테스트 한정으로 assertion을 추가했습니다!
| List<Product> actual = productService.list(); | ||
|
|
||
| // then | ||
| assertAll(() -> { |
| List<MenuGroup> actual = menuGroupService.list(); | ||
|
|
||
| // then | ||
| assertThat(actual).hasSize(3); |
There was a problem hiding this comment.
개수만을 검증하고있네요! contains나 usingRecursive() 등으로 내부까지 검증하는 것이 아니라 개수만으로 충분하다고 생각하신 이유가 궁금해요! 잘못되었다는 것이 아니라 후디의 생각이 궁금합니다!!
There was a problem hiding this comment.
물론 그렇게 객체 내부까지 검사하는것이 가장 좋은 테스트라고 생각합니다! 다만, 재귀적으로 모든 객체의 필드를 검사하는 수고 대비 얻는 이점이 조금 덜하다는 생각이 들어서 사이즈만 검사하도록 하였습니다.
| @Autowired | ||
| private MenuDao menuDao; | ||
|
|
||
| private Menu menu; |
There was a problem hiding this comment.
어떤 필드는 dao를 이용해 DB에 저장된 객체이고, 어떤 필드는 그렇지 않네요. 이 부분을 네이밍에서 구분없이 사용하면 헷갈리지는 않을까요?
There was a problem hiding this comment.
저장된 픽스쳐는 saved, 저장되지 않은 픽스쳐는 created 접두사로 구분하였습니다.
- Menu 도메인 생성자 다시 제거
- MenuGroup 도메인 생성자 다시 제거
- Product 도메인 생성자 다시 제거
- Order 도메인 생성자 다시 제거
- TableGroup 도메인 생성자 다시 제거
- MenuProduct 도메인 생성자 다시 제거
- OrderLineItem 도메인 생성자 다시 제거
- OrderTable 도메인 생성자 다시 제거
|
안녕하세요 아리! 다시 리뷰 요청 드립니다! |
ulimy
approved these changes
Oct 27, 2022
ulimy
pushed a commit
that referenced
this pull request
Nov 8, 2022
* [레거시 코드 리팩터링 - 1단계] 후디(조동현) 미션 제출합니다. (#250) * feat: 자바 버전을 11로 변경 * docs: 요구사항 정리 * test: 테스트 격리를 위한 DatabaseCleaner 추가 * docs: README 포매팅 * test: 상품 서비스 테스트 추가 * test: 메뉴 그룹 서비스 테스트를 통합 테스트로 변경 * test: MenuService 테스트 추가 * test: 누락된 참조 제약 조건 활성화 쿼리 추가 * test: OrderService 테스트 추가 * test: TableService 테스트 코드 추가 * test: TableGroupService 테스트 코드 추가 * test: Menu 픽스처 생성 유틸 클래스 추가 - Menu 도메인 생성자 다시 제거 * test: MenuGroup 픽스처 생성 유틸 클래스 추가 - MenuGroup 도메인 생성자 다시 제거 * test: Product 픽스처 생성 유틸 클래스 추가 - Product 도메인 생성자 다시 제거 * test: Order 픽스처 생성 유틸 클래스 추가 - Order 도메인 생성자 다시 제거 * test: TableGroup 픽스처 생성 유틸 클래스 추가 - TableGroup 도메인 생성자 다시 제거 * test: MenuProduct 픽스처 생성 유틸 클래스 추가 - MenuProduct 도메인 생성자 다시 제거 * test: OrderLineItem 픽스처 생성 유틸 클래스 추가 - OrderLineItem 도메인 생성자 다시 제거 * test: OrderTable 픽스처 생성 유틸 클래스 추가 - OrderTable 도메인 생성자 다시 제거 * test: ServiceTest의 protected 메소드를 사용하여 리팩토링 * test: 엔티티 생성 테스트에서 id가 null이 아닌지 검증하는 assertion 추가 * test: this 키워드 생략 및 저장된 픽스쳐에 대해 saved 접두사 추가 * [레거시 코드 리팩터링 - 2단계] 후디(조동현) 미션 제출합니다. (#319) * docs: 구현 중심으로 적힌 요구사항을 개선 * refactor: DAO 대신 Repository 라는 이름으로 변경 * docs: 요구사항 문서 개선 * chore: jpa.hibernate.ddl-auto를 validate로 설정 - application.properties를 application.yaml로 변경 * test: 메뉴 그룹 인수 테스트 작성 * test: 상품 인수 테스트 작성 * test: 메뉴 테스트 작성 * test: 주문 테이블 테스트 작성 * test: 단체 지정 인수 테스트 작성 * test: 주문 인수 테스트 작성 * test: RequestUtil을 support 패키지로 이동 * test: 깨지는 TableGroup 인수 테스트 수정 * test: 인수 테스트 body를 도메인 대신 Map으로 변경 * refactor: dao 패키지를 repository로 변경 * refactor: MenuGroup request, response DTO 추가 * refactor: Product request, response DTO 추가 * refactor: OrderTable request, response DTO 추가 * refactor: Order request(create, update), response DTO 추가 * refactor: TableGroup request, response DTO 추가 * refactor: Menu request, response DTO 추가 * refactor: 도메인 패키지 분리 * refactor: Menu 도메인이 직접 가격을 검증하도록 개선 * refactor: Product 도메인이 직접 가격을 검증하도록 개선 * test: Nested 테스트 이름 변경 * refactor: 주문한 손님 수 검증을 OrderTable이 수행하도록 개선 * refactor: 주문 항목 및 상태를 Order이 수행하도록 개선 * refactor: OrderStatus의 name()을 사용하지 않고, 열거객체 그대로를 사용하도록 개선 * refactor: MenuGroup의 setter 제거 * refactor: Product의 setter 제거 * refactor: TableGroup의 필요 없는 setter 제거 * test: MenuFixture 제거 * test: MenuGroupFixture 제거 * test: MenuProductFixture 제거 * test: OrderLineItemFixture 제거 * test: Menu의 불필요한 setter 제거 * test: OrderTableFixture 제거 * test: ProductFixture 제거 * test: TableGroupFixture 제거 * test: OrderFixture 제거 * refactor: MenuProduct의 불필요한 setter 제거 * refactor: MenuGroup을 JPA로 리팩토링 * refactor: Product를 JPA로 리팩토링 * refactor: Menu와 MenuProduct를 JPA로 리팩토링 * refactor: MenuProduct의 Long productId 필드를 Product product로 대체 * refactor: Menu의 Long menuGroupId 필드를 MenuGroup menuGroup로 대체 * refactor: MenuProduct의 가격 * 수량과 Menu 가격 검증 로직을 Menu 도메인으로 이동 * refactor: 메소드 추출 리팩토링 * refactor: OrderTable, TableGroup을 JPA로 변경 * refactor: OrderTable 관련 검증 로직을 TableGroup에 이동 * refactor: OrderTable의 TableGroup 및 Empty 설정 로직을 TableGroup에 이동 * refactor: OrderTable 리팩토링 * refactor: Order와 OrderLineItem을 JPA로 이동 * refactor: `@Transactional(readOnly = true)` 어노테이션을 서비스에 추가 * refactor: OrderTable과 TableGroup으로 비즈니스 로직 이동 * refactor: 사용되지 않는 의존성 제거 * refactor: OrderService의 비즈니스 로직을 도메인으로 이동 * refactor: 서비스 코드 리팩토링 * refactor: ungroup 로직을 TableGroupService에서 TableGroup으로 이동 * refactor: 주문 테이블 빈 여부 변경 시 검증 로직을 OrderTable로 이동 * refactor: 테이블 방문한 손님 변경시 검증 로직을 OrderTable로 이동 * refactor: 엔티티 기본 생성자 접근제어자를 protected 로 설정 * refactor: 사용하지 않은 Repository 메소드 제거 * refactor: 메뉴 상품의 수량 * 가격의 합을 검증하는 로직을 리팩토링 * refactor: MenuProduct가 amount를 직접 계산하도록 개선 - Price vo 추가 * refactor: DTO Mapper 추가 - 서비스에서 DTO -> Entity 변환 로직 제거 * refactor: JpaRepository 대신 Repository 를 상속하도록 변경 * refactor: 가격 덧셈 로직 리팩토링 * refactor: 서비스 코드에서만 사용되는 생성자 제거 * refactor: 사용되지 않는 필드 제거 * refactor: 중복 로직 메소드 추출 * refactor: 불필요한 save 로직 제거 * test: 테스트의 불필요한 의존 제거 * chore: TODO 코멘트 추가 * test: saveXXX 메소드가 DTO 대신 도메인을 반환하도록 변경 * refactor: 불필요한 public 생성자의 접근 제어자를 private으로 변경 * refactor: 도메인 생애주기별로 패키지 분리 * refactor: MenuValidator 추가 * refactor: Price의 필드 이름을 value로 변경 * feat: 메뉴가 변경되어도 주문 항목이 변경되지 않도록 개선 * refactor: OrderTable이 TableGroup을 직접참조하는 것을 간접참조로 변경하여 순환참조 제거 * refactor: Order과 OrderTable간의 순환참조 및 직접참조 제거 * refactor: Price를 common 도메인으로 이동 * refactor: 패키지 구조를 도메인 중심으로 변경 * refactor: 의존성 역전을 통한 순환참조 제거
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요, 아리 👋
조금 촉박하게 미션을 진행하느라 테스트 코드가 깔끔하지 않네요 ㅠ-ㅠ 리팩토링 해야겠어요.
아직 도메인 이해도가 많이 낮은 상태에요. 계속 개선하면서 도메인을 파악해보겠습니다!
참고
개선 예정