[codereview]실무에 대한 팁

https://techblog.woowahan.com/2614/

코드리뷰 적응기(feat. 파일럿 프로젝트) | 우아한형제들 기술블로그

 

 

포스팅을 보고 나도 이런 습관이 있나, 고쳐야할게 있나 생각하며 정리해보기로 했다

자세한 설명은 포스팅에 있고 나는 결론적으로 정리할 생각

 

 

 

Entity 

• Lombok @Data 기능 쓰지 말 것, 무분별한 Setter X
• Naming LocalDateTime 타입일 경우 작명 시 SampleDateTime, LocalDate 타입이면 SampleDate (직관적이게)
• Entity에는 LocalDateTIME 값 그대로 두고 DTO 에서 포맷할것
• DETELE 여부는 Boolean 값으로
• 연관관계는 하단에 표시해서 가독성을 높임
• Entity에서는 @NoArgsContructor을 사용해야함. JPA에서는 proxy생성을 위해 기본 생성자를 반드시 필요,  AccessLevel.PROTECTED로 설정하여 JPA에서의 Entity 클래스 생성만 허용

 

동적 QueryDsl

 

이건 예제를 보는게 나을 것 같아서 추가

(아 AS-IS) 나를 보는 것 같아서 슬펐다ㅜㅠ

 

AS-IS

public class SalesRepositoryImpl extends QuerydslRepositorySupport implements SalesRepositoryCustom {
...
    public BooleanBuilder addSearchBuilder(BooleanBuilder builder, QSales sales, SearchType searchType, String searchText) {

        if (!StringUtils.isEmpty(searchText)) {
            if (SearchType.OWNER_ID.equals(searchType)) {
                Long ownerId = Long.valueOf(searchText);
                builder.and(sales.owner.id.eq(ownerId));
            } else if (SearchType.OWNER_NAME.equals(searchType)) {
                builder.and(sales.owner.name.contains(searchText));
            } else if (SearchType.SHOP_NAME.equals(searchType)) {
                builder.and(sales.owner.shopName.contains(searchText));
            }
        }
        return builder;
    }
...
}

 

 

TO-BE

DTO로 받아서 깔끔하게 처리해주기

Querydsl의 BooleanExpression을 null값으로 리턴하면 where 조건문에서 제거됨

public class SalesAdminRepositoryImpl extends QuerydslRepositorySupport implements SalesAdminRepositoryCustom {
...

    private BooleanExpression conditionalOwnerId(SalesSearchRequestDto dto) {
        if (dto.isEmptySearchType() || dto.isNotOwnerId()) {
            return null;
        }
        if (dto.isEmptySearchText()) {
            return null;
        }
        return sales.owner.id.eq(Long.valueOf(dto.getSearchText()));
    }

    private BooleanExpression conditionalOwnerName(SalesSearchRequestDto dto) {
        if (dto.isEmptySearchType() || dto.isNotOwnerName()) {
            return null;
        }
        if (dto.isEmptySearchText()) {
            return null;
        }
        return sales.owner.name.eq(dto.getSearchText());
    }

    private BooleanExpression conditionalShopName(SalesSearchRequestDto dto) {
        if (dto.isEmptySearchType() || dto.isNotShopName()) {
            return null;
        }
        if (dto.isEmptySearchText()) {
            return null;
        }
        return sales.owner.shopName.eq(dto.getSearchText());
    }
...
}

 

 

 

@Autowired 지양

생성자의 인자가 많아지는 경우에 final을 받는 파라미터를 생성자로 만들어주는 @RequiredArgsConstructor을 사용해도 됩니다. 다만 이때는 인자의 추가나 순서 변경시 생성자 클래스가 변경되는 점에 주의해야합니다.

 

 

 

친절한 테스트 코드

@Test
public void PaymentType별_금액_집계() throws Exception {
    //given
    savePayment(PaymentType.CARD, 15000L, sales);
    savePayment(PaymentType.CARD, 13000L, sales2);
    savePayment(PaymentType.POINT, 5000L, sales);
    savePayment(PaymentType.POINT, 8000L, sales2);
    savePayment(PaymentType.COUPON, 9000L, sales2);

    JobParameters jobParameters =
            new JobParametersBuilder()
                    .addString("aggregateDate", localDateToString(sumDate))
                    .toJobParameters();

    //when
    JobExecution jobExecution =  jobTestUtils.getJobTester(JOB_NAME).launchJob(jobParameters);

    //then
    List<PaymentTypeSum> list = paymentTypeSumRepository.findAll();
    assertThat(jobExecution.getStatus(), is(BatchStatus.COMPLETED));
    assertThat(getPaymentTypeSum(list, PaymentType.CARD), is(28000L));
    assertThat(getPaymentTypeSum(list, PaymentType.POINT), is(13000L));
    assertThat(getPaymentTypeSum(list, PaymentType.COUPON), is(9000L));
}

테스트 코드를 짤 때 JUnit에서는 강제하지 않지만 given, when, then 주석을 달고 명확하게 작성해야 합니다.
//given 은 테스트에 필요한 상황, //when 은 테스트하려는 일의 동작, //then 은 when에서 발생한 일의 결과

 

 

 

DTO 잘 사용하기

정말 나도 많이 설계할 때 헷갈렸던 부분, 통일 할 수 있는 걸 최대한 통일시켜보려고 하니 그렇게 하지않는게 좋다고 한다

(엄청 큰 프로젝트이면 더더욱 중요) 작은 프로젝트여도 목적에 따라 DTO를 만들어주는게 좋다고 한다

 

• Dto는 사용에 따라 최대한 쪼개야 합니다. 예를 들어 하나의 Entity를 용도에 따라 보여주는 부분이 다를 때, 화면에서의 리스트 컬럼과 엑셀 다운로드 시 보여줄 컬럼들이 다를 때 필드가 1-2개 달랐지만 하나의 Dto로 사용했었습니다. 각각의 목적에 따라 Dto를 만들어야 합니다.
• 또한, Dto는 용도와 레벨에 맞는 곳에 위치해야 합니다. 저는 dto란 패키지를 만들고 그 안에 모든 dto를 두었었습니다. 하지만 어느 dto는 controller 단까지, 어느 dto는 서비스 레벨까지 사용할 수 있습니다. 이런 용도와 레벨에 맞게 위치시켜야 합니다.

두번째 용도마다 위치 하는 건 또 몰랐네ㅠㅠ 나도 이 부분 적용해봐야겠다