Skip to content
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

[레거시 코드 리팩토링 미션 - 1단계] 우르(김현우) 미션 제출합니다. #493

Merged
merged 13 commits into from
Oct 14, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
34 changes: 34 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,40 @@

## 요구 사항

### 메뉴(Menu)
1. 메뉴를 생성할 수 있다.
- id, name, price, menuGroupId, List MenuProduct
2. 모든 메뉴를 조회할 수 있다.

### 메뉴 그룹(MenuGroup)
1. 메뉴 그룹을 생성할 수 있다.
- id, name
2. 모든 메뉴 그룹들을 조회할 수 있다.

### 주문(Order)
1. 주문을 생성할 수 있다.
- id, orderTableId, orderStatus, orderedTime, orderLineItems
2. 모든 주문을 조회할 수 있다.
3. 특정 주문 상태를 변경할 수 있다.

### 물품(Product)
1. 물품을 생성할 수 있다.
- id, name, price
2. 모든 물품을 조회할 수 있다.

### 주문 테이블 그룹(TableGroup)
1. 여러 OrderTable을 가진 TableGroup을 생성할 수 있다.
- List OrderTable, createdDate, id
2. 특정 TableGroup을 삭제할 수 있다.

### 주문 테이블(Table)
1. 주문 테이블을 생성할 수 있다.
- id, tableGroupId, numberOfGuests, empty(boolean)
2. 모든 주문 테이블들을 조회할 수 있다.
3. 특정 주문 테이블을 empty 상태로 변경할 수 있다.
4. 특정 주문 테이블의 손님의 명 수를 변경할 수 있다.

Choose a reason for hiding this comment

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

요구사항에 예외 케이스에 대한 내용은 없네요...!
저의 경우 서비스의 요구사항엔 예측 가능한 예외 상황에 대한 제약 사항도 포함된다고 생각하는 편인데, 우르는 예외 케이스에 대한 요구사항을 작성하는 것에 대해 어떻게 생각하시나요??

Copy link
Author

Choose a reason for hiding this comment

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

필요하다 생각이 들어서 작성했습니다!!



## 용어 사전

| 한글명 | 영문명 | 설명 |
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/kitchenpos/dao/JdbcTemplateMenuGroupDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ private MenuGroup select(final Long id) {
}

private MenuGroup toEntity(final ResultSet resultSet) throws SQLException {
final MenuGroup entity = new MenuGroup();
entity.setId(resultSet.getLong("id"));
entity.setName(resultSet.getString("name"));
final MenuGroup entity = new MenuGroup(
resultSet.getLong("id"),
resultSet.getString("name")
);
return entity;
}
}
9 changes: 9 additions & 0 deletions src/main/java/kitchenpos/domain/MenuGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ public class MenuGroup {
private Long id;
private String name;

public MenuGroup(final String name) {
this.name = name;
}

public MenuGroup(final Long id, final String name) {
this.id = id;
this.name = name;
}

public Long getId() {
return id;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ logging.level.org.hibernate.type.descriptor.sql.BasicBinder=TRACE
spring.h2.console.enabled=true
spring.jpa.properties.hibernate.format_sql=true
spring.jpa.show-sql=true
spring.flyway.enabled=false

Choose a reason for hiding this comment

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

gradle에 flyway 의존성이 추가되어 있었네요....!
미션 코드들을 굉장히 꼼꼼하게 확인하셨군요👍👍

26 changes: 26 additions & 0 deletions src/main/resources/db/migration/V2__Insert_default_data.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
ALTER TABLE order_line_item ALTER COLUMN seq RESTART WITH 1;
ALTER TABLE menu_product ALTER COLUMN seq RESTART WITH 1;
ALTER TABLE orders ALTER COLUMN id RESTART WITH 1;
ALTER TABLE menu ALTER COLUMN id RESTART WITH 1;
ALTER TABLE product ALTER COLUMN id RESTART WITH 1;
ALTER TABLE order_table ALTER COLUMN id RESTART WITH 1;
ALTER TABLE table_group ALTER COLUMN id RESTART WITH 1;
ALTER TABLE menu_group ALTER COLUMN id RESTART WITH 1;

Choose a reason for hiding this comment

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

오 이렇게 하면 테스트마다 id 값을 일정하게 맞춰줄 수 있겠네요...!
굿입니다👍


insert into product (name, price)
values ('후라이드', 16000);
insert into product (name, price)
Expand Down Expand Up @@ -46,6 +55,9 @@ values (5, 5, 1);
insert into menu_product (menu_id, product_id, quantity)
values (6, 6, 1);

insert into table_group (id, created_date)
values (1, now());

insert into order_table (number_of_guests, empty)
values (0, true);
insert into order_table (number_of_guests, empty)
Expand All @@ -62,3 +74,17 @@ insert into order_table (number_of_guests, empty)
values (0, true);
insert into order_table (number_of_guests, empty)
values (0, true);
insert into order_table (number_of_guests, empty)
values (11, false);
insert into order_table (id, number_of_guests, empty, table_group_id)
values (333, 11, false, 1);
insert into order_table (id, number_of_guests, empty, table_group_id)
values (334, 11, false, 1);
insert into order_table (id, number_of_guests, empty, table_group_id)
values (335, 11, false, 1);

insert into orders
values (1, 'COMPLETION', now(), 1);
insert into orders
values (2, 'COOKING', now(), 2);

45 changes: 45 additions & 0 deletions src/test/java/kitchenpos/application/MenuGroupServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package kitchenpos.application;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.List;
import kitchenpos.domain.MenuGroup;
import kitchenpos.support.ServiceIntegrationTest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

class MenuGroupServiceTest extends ServiceIntegrationTest {

Choose a reason for hiding this comment

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

👍👍👍


@Autowired
private MenuGroupService menuGroupService;

@Test
@DisplayName("create() : 메뉴 그룹을 생성할 수 있다.")
void test_create() throws Exception {
//given
final MenuGroup menuGroup = new MenuGroup("menugroup");

//when
final MenuGroup savedMenuGroup = menuGroupService.create(menuGroup);

//then
assertAll(
() -> Assertions.assertNotNull(savedMenuGroup.getId()),
() -> assertEquals(savedMenuGroup.getName(), menuGroup.getName())
);
}

@Test
@DisplayName("list() : 모든 메뉴 그룹들을 조회할 수 있다.")
void test_list() throws Exception {
//when
final List<MenuGroup> list = menuGroupService.list();

//then
assertEquals(4, list.size());
}
}
72 changes: 72 additions & 0 deletions src/test/java/kitchenpos/application/MenuServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package kitchenpos.application;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.math.BigDecimal;
import java.util.List;
import kitchenpos.dao.MenuProductDao;
import kitchenpos.domain.Menu;
import kitchenpos.support.ServiceIntegrationTest;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

class MenuServiceTest extends ServiceIntegrationTest {

@Autowired
private MenuService menuService;

@Autowired
private MenuProductDao menuProductDao;

@Test
@DisplayName("create() : 메뉴를 생성할 수 있다.")
void test_create() throws Exception {
//given
final Menu menu = new Menu();
menu.setMenuGroupId(3L);
menu.setName("menu1MenuGroup3");
menu.setPrice(BigDecimal.valueOf(13000));
menu.setMenuProducts(menuProductDao.findAll());

//when
final Menu savedMenu = menuService.create(menu);

//then
assertAll(
() -> assertNotNull(savedMenu.getId()),
() -> assertEquals(menu.getName(), savedMenu.getName()),
() -> assertEquals(menu.getMenuGroupId(), savedMenu.getMenuGroupId()),
() -> assertEquals(0, menu.getPrice().compareTo(savedMenu.getPrice()))
);
}

@Test
@DisplayName("create() : 메뉴 가격은 메뉴 상품들의 합보다 크거나 같으면 IllegalArgumentException이 발생할 수 있다.")
void test_create_IllegalArgumentException() throws Exception {
//given
final Menu menu = new Menu();
menu.setMenuGroupId(3L);
menu.setName("menu1MenuGroup3");
menu.setPrice(BigDecimal.valueOf(100000000));
menu.setMenuProducts(menuProductDao.findAll());

Choose a reason for hiding this comment

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

정상 케이스의 Menu 객체를 필드 혹은 Fixture로 만들고
menu.setPrice()로 가격 값을 수정해주면 어떨까요?? 중복되는 부분을 줄일 수 있을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

따로 메서드 분리하였습니다 !


//when & then
Assertions.assertThatThrownBy(() -> menuService.create(menu))
.isInstanceOf(IllegalArgumentException.class);
}

Choose a reason for hiding this comment

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

메뉴의 가격이 0원 미만인 경우와 잘못된 상품 id를 입력 받은 경우도 검증할 수 있으면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

저는 모든 제약조건에 대한 테스트가 존재해야할까 라는 생각입니다.
대부분 레거시 코드를 보게 되면 코드와 테스트 코드를 함께 볼텐데, 모든 제약조건에 테스트가 있으면 무엇이 중요한 테스트인지 보기 어려울 것 같아서요.
그래서 가격이 0미만인 경우나 잘못된 상품 id(dao에서 체크)인 경우에는 예외 테스트 코드를 작성하지 않았습니다.

@Test
@DisplayName("list() : 모든 메뉴들을 조회할 수 있다.")
void test_list() throws Exception {
//when
final List<Menu> menus = menuService.list();

//then
assertEquals(6, menus.size());
}
}
91 changes: 91 additions & 0 deletions src/test/java/kitchenpos/application/OrderServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package kitchenpos.application;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.util.List;
import kitchenpos.dao.OrderDao;
import kitchenpos.dao.OrderLineItemDao;
import kitchenpos.domain.Order;
import kitchenpos.domain.OrderLineItem;
import kitchenpos.domain.OrderStatus;
import kitchenpos.support.ServiceIntegrationTest;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

class OrderServiceTest extends ServiceIntegrationTest {

@Autowired
private OrderService orderService;

@Autowired
private OrderLineItemDao orderLineItemDao;

@Autowired
private OrderDao orderDao;

@Test
@DisplayName("create() : 주문을 생성할 수 있다.")
void test_create() throws Exception {
//given
final Order order = createSuccessfulOrder();

//when
final Order savedOrder = orderService.create(order);

//then
assertAll(
() -> assertNotNull(savedOrder.getId()),
() -> assertEquals(savedOrder.getOrderStatus(), OrderStatus.COOKING.name()),
() -> assertNotNull(savedOrder.getOrderedTime())
);
}

private Order createSuccessfulOrder() {
final OrderLineItem orderLineItem1 = new OrderLineItem();
orderLineItem1.setQuantity(13);
orderLineItem1.setMenuId(1L);
final OrderLineItem orderLineItem2 = new OrderLineItem();
orderLineItem2.setQuantity(3);
orderLineItem2.setMenuId(2L);

final Order order = new Order();
final long orderTableId = 335L;
order.setOrderTableId(orderTableId);
order.setOrderLineItems(List.of(orderLineItem1, orderLineItem2));
return order;
}

Choose a reason for hiding this comment

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

여기도 비어있는 테이블에서 주문을 한 경우(=OrderTable.isEmpty()==true), 주문 목록(OrderLineItems)이 비어있는 경우, 잘못된 메뉴&테이블이 포함되어 있는 경우에 대한 검증 테스트가 있으면 좋을 것 같아요~!

Copy link
Author

Choose a reason for hiding this comment

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

비어있는 테이블에서 주문을 한 경우 테스트 추가했습니다 !


@Test
@DisplayName("list() : 모든 주문들을 조회할 수 있다.")
void test_list() throws Exception {
//given
final int beforeSize = orderService.list().size();

orderService.create(createSuccessfulOrder());

//when
final List<Order> savedOrders = orderService.list();

//then
assertEquals(savedOrders.size(), beforeSize + 1);
}

@Test
@DisplayName("changeOrderStatus() : 주문 상태를 변경할 수 있다.")
void test_changeOrderStatus() throws Exception {
//given
final long savedOrderId = 3333L;

final Order order = new Order();
order.setOrderStatus(OrderStatus.COMPLETION.name());

//when
final Order updatedOrder = orderService.changeOrderStatus(savedOrderId, order);

//then
assertEquals(order.getOrderStatus(), updatedOrder.getOrderStatus());
}

Choose a reason for hiding this comment

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

이미 계산이 완료된 주문의 주문상태는 수정할 수 없게 구현이 되어 있어서 이부분에 대해서도 검증 테스트가 있으면 좋을 것 같습니다!

}
62 changes: 62 additions & 0 deletions src/test/java/kitchenpos/application/ProductServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package kitchenpos.application;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.math.BigDecimal;
import java.util.List;
import kitchenpos.dao.ProductDao;
import kitchenpos.domain.Product;
import kitchenpos.support.ServiceIntegrationTest;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

class ProductServiceTest extends ServiceIntegrationTest {

@Autowired
private ProductService productService;

@Autowired
private ProductDao productDao;

@Test
@DisplayName("create() : 물품을 생성할 수 있다.")
void test_create() throws Exception {
//given
final Product product = new Product();
product.setName("product");
product.setPrice(BigDecimal.valueOf(10000));

//when
final Product savedProduct = productService.create(product);

//then
assertAll(
() -> assertNotNull(savedProduct.getId()),
() -> assertEquals(0, savedProduct.getPrice().compareTo(product.getPrice())),
() -> assertEquals(savedProduct.getName(), product.getName())
);
}

Choose a reason for hiding this comment

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

상품 가격이 0원 미만인 경우에 대한 검증 테스트도 있으면 좋을 것 같아요!


@Test
@DisplayName("list() : 모든 물품을 조회할 수 있다.")
void test_list() throws Exception {
//given
final Product product = new Product();
product.setName("product");
product.setPrice(BigDecimal.valueOf(10000));

final int beforeSize = productService.list().size();

productDao.save(product);

//when
final List<Product> products = productService.list();

//then
assertEquals(products.size(), beforeSize + 1);
}
}
Loading