feat : 상품 CRUD 기능 구현 - [merged]

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:52

_Merges feature/itemcrud -> develop

@haaeee @rkdud1108 @AnTaeWook @halucinor

작업 내용

issue #10

상품 CRUD 기능 구현

• 상품 조회
• 상품 생성
• 상품 수정
• 상품 제거
• 상품 목록 조회

미구현
- 상품 목록 조회(TBD)

스크린샷

Build

TEST

• [x] 💯 테스트는 잘 통과했나요?
• [x] 🏗️ 빌드는 성공했나요?
• [x] 🧹 불필요한 코드는 제거했나요?
• [x] 💭 이슈는 등록했나요?
• [x] 🏷️ 라벨은 등록했나요?
• [x] 💻 git rebase를 사용했나요?

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:52

requested review from @rkdud1108

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 💯 테스트는 잘 통과했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 🏗️ 빌드는 성공했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 🧹 불필요한 코드는 제거했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 💭 이슈는 등록했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 🏷️ 라벨은 등록했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

marked the checklist item 💻 git rebase를 사용했나요? as completed

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 13:53

상품 페이징(목록 조회) 기능 구현 중 입니다 :D

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 14:49

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 43

이 부분은 아직 테스트 코드 작성 부분이 아닌건가요?

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 14:50

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 66

final keyword 붙이는 거 수정하셔야 될 것 같아요!

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 14:50

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 40

DeleteMapping을 하고 HttpStatus.NO_CONTENT로 하셨는데 차이가 어떤게 있을까요??

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:07

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 43

작업 후 push 예정입니다~

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:08

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 66

넵 확인했습니다

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:10

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 40

다른 API 처럼 아래 같은 형태를 사용하고 싶었는데 방법을 아직 못찾아 일단 이렇게 구현해습니다.

ResponseEntity.ok().body();

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 15:11

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 43

알겠습니다!

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 15:12

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 40

return ResponseEntity.noContent() .build();

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 15:12

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 40

백커님이 구성하신게 좋은 것 같습니다!

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 15:13

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 14

@RestController로 변경하셔야될것같아요!

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:15

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 43

changed this line in version 2 of the diff

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:15

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 66

changed this line in version 2 of the diff

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:15

added 3 commits

• 60e8fdc2 - feat : 상품 CRUD 목록 조회 기능 구현
• 2de07f16 - feat : 상품 CRUD 기능 final 키워드 추가
• 222c5ca6 - feat : 상품 CRUD 기능 테스트 구현

Compare with previous version

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:22

상품 CRUD 기능 구현 마무리 하였습니다.

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:39

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 14

changed this line in version 3 of the diff

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:39

added 1 commit

• 4c80e195 - feat : 상품 CRUD 기능 final 키워드 추가

Compare with previous version

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 15:42

added 1 commit

• ecdbdcf5 - feat : 상품 CRUD 기능 ItemRepository 주석제거

Compare with previous version

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:06

Commented on src/main/java/com/gabia/bshop/dto/ItemDto.java line 17

추후에 확장하시기 위해서 DTO를 범용적이게 두고 사용하시는 걸까요?
엔티티를 반환하는 것과 다를 것이 없어보입니다.

DTO는 화면단과 밀접한 관계가 있는 클래스라고 생각하는데, mock 데이터를 반환하는 로직이어도 반환데이터는 명시적으로 필요한 필드만 선언해서 사용하는 것을 추천드립니다.

추가적으로 deleted 컬럼은 운영용 데이터인데, 화면단까지 노출되는 경우는 없을 것 같습니다.
단순히 SQL 조회시에 필터속성을 거는 친구이고, 화면단에서 데이터를 받는 클라이언트는 item이 deleted 값의 여부를 신경 쓸 필요가 없다고 생각합니다!

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:09

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 27

이건 진짜 개인 취향인데요, get 접두사는 데이터클래스의 getter와 혼동되는 경우가 있어서 저는 선호하지는 않습니다.
서비스로직이 뭘 하는지를 명시적으로 나타내주는게 메서드 이름인 것 같은데요, 저는 그래서 find접두사를 좋아하는 편입니다.

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 16:13

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 14

변경했습니다~

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 16:19

Commented on src/main/java/com/gabia/bshop/dto/ItemDto.java line 17

네 아직 구체화 되지 않은 부분에 있어 향후 기능 수정을 위해 일단 ItemDTO를 범용적으로 사용하고 있습니다. 나중에 적절하게 Dto를 세분화 할 생각입니다. 그리고, 말씀하신 것처럼 deleted 필드는 제거하는게 좋을거 같아요

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 16:20

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 27

제안해주신 것처럼 find로 메서드 명칭을 변경했습니다. 훨씬 좋은거 같아요

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 16:23

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 27

changed this line in version 5 of the diff

[halucinor]

In GitLab by @halucinor on Feb 3, 2023, 16:23

added 3 commits

• 2a89e38d - style : get -> find 로 메서드 명칭 변경
• f81fd762 - feat : itemDto deleted 필드 제거
• 58dcfedf - style : itemDto spotlessApply

Compare with previous version

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 39

• List와 Items라는 명칭이 충돌하는 것 같습니다. 둘 다 컬렉션을 의미하는데 이 컨벤션대로라면 getListItem이나 getItems가 더 좋을 것 같습니다
• 메서드나 변수 네이밍에는 자료구조를 유추할 수 있는 네이밍은 좋지 않다고 합니다. 추후 반환되는 자료구조가 변경되었을 때 이름까지 함께 바꿔줘야 하는 포인트가 추가적으로 생긴다고 하더라구요. 중요한건 아닙니다
• 상품 상태에서 deleted 필터링 조건이 쿼리에 걸려있지 않는 것으로 보입니다.

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 23

실수시겠지만 PageItem() 카멜케이스가 적용되어있지 않습니당

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/main/java/com/gabia/bshop/entity/Item.java line 85

update() 메서드를 정의해서 여러 용도로 사용하시려는 걸까요? 개인적인 생각은, 필요한 범위의 변경만 정의된 특화된 update 메서드를 만들어서 사용하는 것이 더 좋다고 생각합니다.
전체 컬럼을 업데이트해야 할 요건은 제 생각엔 없을 것 같다는 생각이 들어요. 있어도 개별의 메서드를 여러번 호출하는 것이 더 명시적일 것 같습니다.
하지만\~ 여전히 개인적인 의견입니다\~

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 90

이렇게 beforeEach에 when() 구문을 추가해도 작동되겠지만, 실제 각 테스트에서 흐름을 따라가기 힘든 것 같습니다.
개인적으로 beforeEach나 afterEach 같은 테스트 셋업 같은 메서드들은 리소스의 생성과 해제에 집중하고 실제 행위에 대한 검증은 각 테스트에서 담당하는 것이 더 좋다고 생각합니다.

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 98

• 말씀드렸던 사항인데, when() 구문이 beforeEach에 존재해서 테스트가 명시적으로 어떤 것을 검증하는지 알기 힘든 것 같습니다
• '~~을 했을 때 ~~하면 ~~한다' 같은 테스트명으로 변경하면 가독성도 좋고 테스트를 이해하는 데 더 도움이 될 것 같습니다.

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 136

여기서 when(itemRepository.save(item)).thenReturn(item); 구문은 //when 에 속하는게 맞을까요?
그리고 ItemDto changeedItem = itemService.updateItem(itemDto); 구문도 //then 에 속하는게 맞을까요? given when then 패턴에 대해서 다같이 공부하면 좋을 것 같습니다.

제 생각엔 128라인까지는 테스트를 준비하는 과정이라서 //given 의 스코프
131라인은 실제 검증하려는 행위이므로 //when 스코프 나머지 assert는 //then 스코프인 것 같습니다

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 196

비슷한 리뷰인데요, //then 에서 행위와 검증을 동시에 진행하는 것 같습니다.
저는 이럴 땐 //when & //then 처럼 함께 기재하는 편입니다

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 199

given when then 컨벤션이 존재했으면 좋겠습니다

[halucinor]

In GitLab by @AnTaeWook on Feb 3, 2023, 16:23

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 215

개인적인 경험으로는, 사용되지 않는 코드를 주석해뒀다가 나중에 다시 주석을 해제하고 활용하는 경우가 매우 적었습니다.
주석코드는 지우는 것을 추천드립니다

[halucinor]

In GitLab by @haaeee on Feb 3, 2023, 16:32

Commented on src/main/java/com/gabia/bshop/entity/Item.java line 85

update() 메서드를 정의해서 여러 용도로 사용하실 때 내부 세부적으로 필요한 범위를 나눠서 정리하는 것도 더 좋다고 생각됩니다.

예를 들어

public void update(final Member updateMember) {
        updateName(updateMember.name);
        updateImageUrl(updateMember.imageUrl);
        updateRegistered(updateMember.registered);
    }

    private void updateName(final String name) {
        if (name != null) {
            this.name = name;
        }
    }

    private void updateImageUrl(String imageUrl) {
        if (imageUrl != null) {
            this.imageUrl = imageUrl;
        }
    }

    private void updateRegistered(final boolean registered) {
        if (this.registered) {
            return;
        }
        this.registered = registered;
    }

[halucinor]

In GitLab by @rkdud1108 on Feb 5, 2023, 19:03

Commented on src/main/java/com/gabia/bshop/mapper/ItemMapper.java line 18

Mapping부분에 source = "category", target = "categoryDto" 이렇게 바꾸는게 좋을 듯 합니다~

[halucinor]

In GitLab by @rkdud1108 on Feb 5, 2023, 19:25

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 53

itemDtoToEntity 메소드에 source, target 설정 해주면 따로 addCategory라는 로직을 만들지 않아도 매핑 되어 들어갑니다.
이런 방식으로 수정하는 게 좋을 거 같습니다.

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 40

changed this line in version 6 of the diff

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/main/java/com/gabia/bshop/dto/ItemDto.java line 17

changed this line in version 6 of the diff

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/main/java/com/gabia/bshop/service/ItemService.java line 39

changed this line in version 6 of the diff

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/main/java/com/gabia/bshop/controller/ItemController.java line 23

changed this line in version 6 of the diff

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/main/java/com/gabia/bshop/entity/Item.java line 85

changed this line in version 6 of the diff

[halucinor]

In GitLab by @halucinor on Feb 6, 2023, 10:37

Commented on src/test/java/com/gabia/bshop/service/ItemServiceTest.java line 90

changed this line in version 6 of the diff