Skip to content

#22 상품 생성 기능 개발#29

Open
LeeJaeYun7 wants to merge 9 commits into
mainfrom
feat/product
Open

#22 상품 생성 기능 개발#29
LeeJaeYun7 wants to merge 9 commits into
mainfrom
feat/product

Conversation

@LeeJaeYun7
Copy link
Copy Markdown
Collaborator

@LeeJaeYun7 LeeJaeYun7 commented Sep 28, 2023

  • 상품(Product) 생성 기능을 추가했습니다.
  • 상품은 사용자가 구입할 수 있는 패키지를 의미합니다.
    image

@LeeJaeYun7 LeeJaeYun7 changed the title #22 상품 생성 기능 추가 #22 상품 생성 기능 개발 Sep 28, 2023
Copy link
Copy Markdown
Collaborator

@f-lab-moony f-lab-moony left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다 ~
코멘트 확인 후 리뷰 재요청 해주세요 ~


@Builder
public Interview(Long id, Member member, List<Answer> answers) {
public Interview(Member member, Member mentor, Product product, Long memberLevel, Long mentorId, LocalDateTime interviewDateTime) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mentorId는 왜 받는걸까요 ?

@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

int type;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

접근지정자가 없는 이유가 있을까요 ~?

@@ -0,0 +1,10 @@
package com.example.interviewPrep.quiz.product.domain;

public class ProductOne extends Product{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

상속을 사용한 이유가 있을까요 ?

import java.time.LocalDateTime;

@Getter
public class ProductRequest {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

테스트에서 사용하려면 생성자도 필요하지 않을까요 ~?


public void createProduct(ProductRequest productRequest){

int type = productRequest.getType();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

타입이 숫자형으로 관리되는건 가독성과 안정성이 전부 떨어질 것 같은데, 다른 방법을 고려한다면 어떤 방법들이 있을까요 ?

@@ -0,0 +1,16 @@
package com.example.interviewPrep.quiz.utils;

public enum InterviewCnt {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

클래스 이름이 무슨 뜻을 가지고 있는걸까요 ? 그리고 줄임말은 자제해주시는게 좋습니다 클래스 이름이 길어지더라도요!


import lombok.Getter;

@Getter
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

enum에 @Getter를 붙이면 무슨 메서드가 생성되나요 ?

@Getter
public enum ProductType {

ONE(1), TWO(2), THREE(3);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

타입에 이름을 붙이는 건 어떨까요 ? 지금은 구분이 크게 의미가 없는 것 같은데 ..
그리고 이 enum이 어디서 쓰이는거에요 ?


@Test
@DisplayName("유효한 답안 생성")
void createValidProduct() throws Exception {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

throws 필요 한가요 ?

mockMvc.perform(post("/api/v1/products")
.accept(MediaType.APPLICATION_JSON)
.contentType(MediaType.APPLICATION_JSON)
.content(validJsonRequest))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

지금은 크게 안느껴지겠지만, 테스트 안에 요청에 대한 정의가 없어서 테스트가 많아지면 무슨 요청이 가는지가 안보여서 어떤 상황에서 이 테스트가 동작하는지를 확인하려면 setup까지 계속 왔다갔다 해야될 것 같은데요, 저는 직접적으로 when 절에서 필요한 요청에 대해서는 given절에 선언하는게 테스트 가독성이 높아진다고 생각합니다.

재윤님은 setup에서 정의하는게 더 낫다고 생각하시는걸까요 ?

LeeJaeYun7 added a commit that referenced this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants