[review] f1-washpedia #144
Conversation
YunNote
commented
Feb 17, 2024
YunNote
commented
- 리뷰 진행
YunNote
requested review from
linglong67,
HyunJunSon,
chan99k and
gunsight1
as code owners
| @@ -20,8 +20,12 @@ | |||
| public class CommonCodeController { | |||
|
|
|||
| private final CommonCodeService commonCodeService; | |||
|
|
|||
| // | |||
There was a problem hiding this comment.
만약 버저닝을 해야한다면 어떻게 관리할 수 있을까요?
URL로도 관리할 수 있지만 header를 통해서도 관리하는 방법에 대해 보시면 좋을것 같습니다!
| @GetMapping("/{codeName}") | ||
| public ResponseEntity<ApiResponse<List<CommonCodeDto>>> getCommonCode (@PathVariable String codeName){ | ||
|
|
||
| // | ||
| List<CommonCodeDto> codes = commonCodeService.getCodes(codeName); |
There was a problem hiding this comment.
반환타입을 저장하는 변수를 만드는것도 좋지만 사용처가 없다면 바로 return 문에 넣어주는것은 어떨까요 ?return
ApiResponse.toResponseEntity(GET_COMMON_CODE_SUCCESS, commonCodeService.getCodes(codeName));
| @@ -17,6 +17,7 @@ public class CommonCodeService { | |||
|
|
|||
| public List<CommonCodeDto> getCodes(String codeName) { | |||
|
|
|||
| // | |||
| return commonCodeRepository.findAllByUpperNameAndIsUsed(codeName,true) | |||
There was a problem hiding this comment.
항상 true만 반환하는데 spring data jpa 에서 제공하는 true를 그냥 써도 될것같습니다!
참고 사이트 전달드립니다
| @@ -25,6 +25,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons | |||
| boolean result = true; | |||
| String requestToken = request.getHeader("Authorization"); | |||
|
|
|||
| // | |||
| if (requestToken == null || requestToken.isEmpty()) { throw new BusinessException(AcceptInterceptorErrorCode.DOSE_NOT_EXIST_REQUEST_TOKEN); } | |||
There was a problem hiding this comment.
Objects.isNull 을 활용하여 null에 대해 명시적으로 관리하면 좋을것 같습니다! 또한 2가지 조건에대해서도 처리 가능합니다!
module-api/src/main/java/com/kernel360/main/controller/MainContoller.java
Show resolved
Hide resolved
| @@ -124,6 +125,8 @@ public boolean idDuplicationCheck(String id) { | |||
|
|
|||
| @Transactional(readOnly = true) | |||
| public boolean emailDuplicationCheck(String email) { | |||
|
|
|||
| // | |||
| Member member = memberRepository.findOneByEmail(email); | |||
|
|
|||
| return member != null; | |||
There was a problem hiding this comment.
해당 null처리 하는 부분도 여러가지로 보면 좋을듯 합니다.
- return Objects.nonNull(member);
- return memberRepository.existMemberByEmail(email) 과 같이 메서드 레벨 반환
| @@ -200,6 +204,7 @@ public Map<String, Object> getCarInfo(String token) { | |||
|
|
|||
| @Transactional | |||
| public void saveWashInfo(WashInfoDto washInfoDto, String token) { | |||
| // | |||
There was a problem hiding this comment.
POST 및 PUT에 대해서 전반적인 값에 대한 validation처리가 필요할듯합니다.
등록 및 수정에서 올바르지 않은 값들이 DB에 들어가면 원하지 않은 에러들을 마주할 확률이 높습니다.
따라서 등록 및 수정에서는 validation처리가 타이트하게 들어가는것이 좋을것 같습니다
| @@ -19,6 +19,7 @@ public class ProductController { | |||
|
|
|||
| private final ProductService productService; | |||
|
|
|||
| // | |||
| @GetMapping("/products") | |||
| ResponseEntity<ApiResponse<List<ProductDto>>> findProductList(){ | |||
There was a problem hiding this comment.
ProductDto를 2개 이상의 컨트롤러에서 같이 사용하고 있습니다.
해당 Response에서 고려해보면 좋을만한 내용은 다음과 같습니다.
- API가 개발진행됨에 따라 2개의 API가 동일한 방향으로 Response가 개발되는지?
만약 조금이라도 다른방향으로 개발이 진행된다면 해당 API는 변경이 필요할것 같습니다!.
저는 개인적으로 API와 Request,Response는 한쌍으로 작업하는 편입니다! 참고부탁드립니다!
| @@ -16,6 +16,8 @@ public class Auth extends BaseEntity { | |||
| @Id | |||
| @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "auth_id_gen") | |||
| @SequenceGenerator(name = "auth_id_gen", sequenceName = "auth_auth_no_seq") | |||
|
|
|||
| // | |||
| @Column(name = "auth_no", nullable = false) | |||
There was a problem hiding this comment.
JPA기본 설정시 underscore로 할지 camelcase로 컬럼명을 지정할지 정의할 수 있습니다
해당 옵션을 확인해보면 @column을 줄일 수 있을듯 합니다.
| @@ -13,6 +13,7 @@ | |||
|
|
|||
| @Getter | |||
| @MappedSuperclass | |||
| // 중복 | |||
| @EntityListeners(AuditingEntityListener.class) | |||
There was a problem hiding this comment.
해당 설정이 중복되어 설정되어있는듯합니다.
spring application root에 설정 or configuration 파일에 단건 설정해주시는것도 좋을듯합니다.
