[카카오테크캠퍼스] 2주차 과제 코드리뷰 정리

Back-end/카카오테크캠퍼스

[카카오테크캠퍼스] 2주차 과제 코드리뷰 정리

잔디🌿 2024. 7. 11. 16:27

1. 페이지 응답을 담당하는 핸들러와 클라이언트의 요청을 받는 핸들러를 분리하자

처음에는 한 엔티티에 대한 컨트롤러가 하나여야한다고 생각했어서 어떤 것을 고쳐야하는지 잘 몰랐었는데, 팀원분들께 물어보니까 컨트롤러를 2개 만들어도 된다고 하셨다. 그래서 @RestController을 쓰는 컨트롤러, @Controller을 쓰는 컨트롤러 이렇게 2가지를 만들었다.

@Controller
@RequestMapping("/menus/view")
public class MenuController {

    private final MenuService menuService;

    public MenuController(MenuService menuService) {
        this.menuService = menuService;
    }

    public String returnView(
            String errorMsg,
            Model model) {
        if (errorMsg != null) {
            model.addAttribute("errors", errorMsg);
            model.addAttribute("menus", menuService.findall());
            return "Menu";
        }
        model.addAttribute("menus", menuService.findall());
        return "redirect:/menu";

여기는 view를 반환한다. 위와 같이 모델에 값을 넣는 작업도 필요 없이 빈 깡통뷰만 리턴해도 된다고 하셨다.

@RestController
@RequestMapping("/menus")
public class MenuRestController {

    private final MenuService menuService;

    public MenuRestController(MenuService menuService) {
        this.menuService = menuService;
    }

    @PostMapping
    public ResponseEntity<Object> save(
            @ModelAttribute @Valid MenuRequest request,
            BindingResult result
    ) {
        if(result.hasErrors()){
            return ResponseEntity.badRequest().body(result.getFieldError().getDefaultMessage());
        }
        else{
            MenuResponse menuResponse =  menuService.save(request);
            return ResponseEntity.ok().body(menuResponse);
        }
    }

여기는 ResponseEntity를 반환한다.

2. 계층별 유지보수 범위를 전파시키고 싶지 않기 위해서 요청을 전달하는 객체와 비지니스 로직을 수행할 객체를 분리하자

엔티티와 Response, Request 이렇게 세개의 요소를 만들었다.

Response와 Requset는 recode를 사용하였다.

3. @ControllerAdvice @RestControllerAdvice 의 차이

ControllerAdvice는 주로 view를 반환할 때 쓰이고, RestControllerAdvice은 주로 JSON형태로 반환할 때 쓰인다고 합니다.
제 코드에서 RestControllerAdvice를 썼음에도 오류가 나지 않았던 이유를 찾아봤는데 ResponseEntity로 반환하는 과정에서 자동으로 JSON으로 변환된다고 하였습니다!
하지만 제 코드의 가독성을 위해서는 RestControllerAdvice를 사용하는 것이 좋다고 생각되어 이 부분은 수정하였습니다!

4.MethodArgumentNotValidException 외에도 여러가지 예외가 다양하게 발생할 수 있는데,
그것들은 모두 어떻게 처리해줄 수 있을까요?

package gift.exception;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

import java.util.HashMap;
import java.util.Map;

@RestControllerAdvice
public class EtcExceptionHandler {
    @ExceptionHandler(Exception.class)
    public ResponseEntity<Map<String,String>> handleEtcExceptions(
            Exception exception
    ){
        Map<String,String> errors = new HashMap<>();
        errors.put("errerEtc",exception.getMessage());
        return ResponseEntity.badRequest().body(errors);
    }
}

위와 같이 ExceptionHandler(Exception.class)를 사용해서 설정한 에러 이외의 오류를 처리하였다.

5. 필수값 여부로 NOT NULL 설정을 추가

null이면 안되는 필드에도 위 조건을 추가하지 않았었다. 앞으로는 더 신경써야겠다.

6. JWT토큰은 헤더에

이건 내가 PR에 여쭤본 부분이다.

구현 방식은 자유지만, 보편적으로 사용되는 방식들이 있는데, 이걸 통신규약이라고 말한다.

HTTPS 를 통해 전송되는 헤더는 암호화되어 전송된다. Authorization 헤더를 중간에 가로채더라도 볼 수 없기 때문에 통신규약에 따르면 보안상 이점을 더욱 누리기 위해 header 에 담는 것을 권장한다.

7. 축약어 사용은 지양하자

나는 passwd라는 축약어를 사용했는데 이건 가독성에 좋지 않다고 한다. 그래서 모두 password로 바꿨다.

8. Controller은 클라이언트에게 정보를 전달하는 기능만 집중하도록 하자

나는 비밀번호 일치 여부를 Controller단에서 확인했는데 이러면 controller이 역할에 집중할 수 없다고 한다. 그래서 이 부분을 service가 수행하는 것으로 바꾸었다.

9. 접근제한자를 명시하자

접근제한자를 빼먹었다. 앞으로 리뷰요청하기 전에 이 부분 확인해야겠다.

10. 파일의 끝에 개행문자 넣어주기

나는 위와 같이 파일의 끝에 개행문자를 추가하지 않았었다. 그런데 이렇게 하면 파일이 끝났음을 알릴 수 없어 EOF를 할 수 없다고 한다. 그래서 나는 인텔리제이에서 위 기능을 자동으로 수행해주는 부분을 설정하였다. 이 기능을 설정해주고 저장을 하면 자동으로 반영된다.

멘토님이 블로그 링크를 주셨다!

https://hyeon9mak.github.io/github-no-newline-at-a-end-of-file/

GitHub 파일 끝에 개행이 필요한 이유 (no newline at a end of file)

GitHub 에 Pull Request 를 올리면 가끔 아래와 같은 금지 마크를 발견할 때가 있다.

hyeon9mak.github.io

11. url 이름과 메서드 이름을 동일하게 하자

@PostMapping("/register")
    public ResponseEntity<String> join(
            @RequestParam("id") String id,
            @RequestParam("password") String password
    ) {
        MemberRequest memberRequest = new MemberRequest(id,password);
        return memberService.join(memberRequest);
    }

처음에는 위와 같이 구현하였는데, 이런 사소한 포인트로 동료 개발자들의 생산성에 차이가 생긴다고 한다.

바로 수정했다

12. 경로명은 복수형으로 하자

@RestController
@RequestMapping("/menus")
public class MenuRestController {

    private final MenuService menuService;

    public MenuRestControl

URL 경로를 볼 때 만약 "/menus/{id}' 이런 식으로 받으면 메뉴들 중 해당 아이디에 해당하는 것을 ..해라 이렇게 해석한다고 한다

이를 용이하게 하기 위해서는 복수형을 쓰는 것이 좋다.

13. @ModelAttribute와 @RequsetBody의 차이점은?

ModelAttribute는 눈에 보이는 빈 생성자가 있다면 무조건 setter가 있어야 한다.

이때문에 내가 Recode를 써도 오류가 나지 않았던 것이다.

반면 RequestBody는 setter가 없어도 된다.

14. 옳지 않은 부분이 생기면 null반환등의 기능을 넣지 않고, 바로 Exception을 발생시킨다.

나는 원래 null을 반환하여 이걸 받으면 예외처리를 하도록 하였는데, 그냥 그 자리에서 Exception을 발생시키고 Exception핸들러가 이를 처리하도록 하는 것이 좋다.

15.ResponseEntity를 다루는 것은 Controller에서!

나는 완성된 객체를 반환해야한다는 생각에 ResponseEntity도 Service에서 다루도록 하였는데, 이 부분은 Controller의 역할이라고 한다.

16. 불필요한 import 다 지우자

import 남겨두는게 괜찮다고 생각했는데 다른 개발자가 볼 때 혼란스러울 수 있다고 한다.

패키지를 클릭하고 Control + option + o 하면 된다고 하셨다.

17. DTO -> ENTITY 매핑하는 부분 엔티티나 Controller 부분에 Static로 선언하는 방식도 있다.

이 부분은 필드값이 있는 것이 아니고, 단순히 구현만 하므로 static으로 선언하여 사용해도 된다고 하셨다

18. 에러메세지를 바로 반환하지 않고 커스텀메세지를 쓰자

나는 에러를 처리할 때 에러메세지를 있는 그대로 클라이언트에게 전달했는데, 이렇게 하면 보안상의 이유로 좋지 않아서

"예상치 못한 오류가 발생하였습니다. 다시 시도해주세요"와 같은 커스텀 메세지를 사용하는게 좋다고 하셨다

19.Optional을 쓰자

Optional을 쓰면 null도 처리할 수 있고, 빈 값이 왔을 때 바로 exception을 날릴수도 있다고 한다.

20. PatchMapping이란?

PatchMapping이란 리소스의 일부를 수정하는 것이다.

put과는 달리 리소스의 전체를 교체하지 않는다.

만약 name만 변경하고싶다면

@PatchMapping("api/name/{id}")

이런식으로 설정하고 해당 메서드에서 name만 수정하고 이를 Repository에 보내면 된다.