Skip to content

3단계 - @MVC 구현(힌트) - 리뷰요청#245

Merged
changgunyee merged 23 commits intonext-step:headf1rstfrom
headF1rst:step3
Aug 26, 2022
Merged

3단계 - @MVC 구현(힌트) - 리뷰요청#245
changgunyee merged 23 commits intonext-step:headf1rstfrom
headF1rst:step3

Conversation

@headF1rst
Copy link

안녕하세요 창권님! 3단계 힌트를 적용하여 기존 코드를 개선해 보았습니다.
항상 다양한 의견 주셔서 감사하고 이번에도 세세한 피드백 해주시면 감사하겠습니다 🙇‍♂️

이전 미션에서 주신 피드백도 함께 반영하였으며 일부 피드백에 대한 제 의견을 아래에 남겨보았습니다.

  • HandlerMapping 인터페이스의 getHandler() 메서드 반환형을 Object 보다는 인터페이스나 클래스를 반환해 볼 수 있을지 주신 의견.
    ->
    제가 알기로는 Handler는 좀 더 넓은 범위의 Controller를 의미한다고 알고있습니다.
    때문에 반환형을 Controller 인터페이스로 정의하는 것은 Handler라는 의미에 적합하지 않다고 생각하였고 HandlerExcution를 반
    환하는것 또한 RequestMapping을 사용 못하게 된다는 점이 걸렸습니다.
    또한 새로운 Mapping이 추가될 확장 가능성을 염두하여 반환형을 Object로 유지하는 선택을 해보았습니다.

Copy link

@changgunyee changgunyee left a comment

Choose a reason for hiding this comment

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

안녕하세요. 리뷰어 이창권입니다.

피드백 잘 반영해주셨습니다.
추가로 제가 반환형을 일치시키는 것을 제안드렸는데 Adapter클래스들을 만들어서 해결하셨네요!
각각의 장단점을 이해하셨으면 충분하다고 생각합니다.

또한 피드백 몇가지 드렸는데 중요하지만 많지는 않아 다음 단계 진행 전에 꼭 작업해주셨으면 합니다.
다음 단계로 넘어가시죠 🚀

import java.lang.reflect.Method;

public class HandlerExecution {
public class HandlerExecution implements HandlerAdapter {

Choose a reason for hiding this comment

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

클래스 구조상 HandlerExecution, Controller가 비슷한 역할을 하는 클래스입니다.
따라서 HandlerExecution를 실행시키기 위한 HandlerAdapter는 새로운 클래스로 가져가는게 더 확장성이 좋아보여요.

String requestURI = request.getRequestURI();
RequestMethod requestMethod = RequestMethod.valueOf(request.getMethod().toUpperCase());
return handlerExecutions.get(new HandlerKey(requestURI, requestMethod));
private void initHandlerExecution(Map<Class<?>, Object> controllers) {

Choose a reason for hiding this comment

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

List을 반환하게 해서 initMapping()에서 handlerExecutions에 추가해주는건 어떨까요?
좀 더 가독성이 좋을 것 같네요 😄

@Override
public void init() {
handlerAdapterStorage = new HandlerAdapterStorage();
handlerAdapterStorage.init();

Choose a reason for hiding this comment

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

생성자가 아닌 init 메서드들을 만드시는 이유를 여쭤봐도 될까요?
생성자에서 하면 필드의 final을 더 적극적으로 이용할 수 있는 이점이 있을 것 같아서요.
예를 들어 생성자를 이용한다고하면 handlerAdapters를 add하는 방식이 아닌 생성자에서 만들고 바로 할당할 수 있으니까요!

Copy link
Author

Choose a reason for hiding this comment

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

창권님 말씀대로 생성자를 사용함으로써 얻는 이점이 많을거 같네요! 피드백 해주신 방법으로 수정해 보도록 하겠습니다. 🙇‍♂️

이번 미션을 진행하면서 디스패처 서블릿에 대해서 공부를 해보았는데요.

미션 진행 당시에 저는 init() 메서드가 서블릿 컨테이너에 의해서 딱 한번 호출이 되며, 호출되면서 필드값에 값을 주입하게 된다고 이해하였습니다.
Servlet (Java EE 6)

때문에 생성자가 아닌 init() 메서드에서 필드값을 초기화 해주었는데요.

사실 아직 서블릿 컨테이너에서 어떤 방식으로 디스패처 서블릿을 초기화 시켜주는지 많이 헷갈리기 때문에 관련 포스팅이나 자료가 있으시다면 알려주시면 감사하겠습니다!

Choose a reason for hiding this comment

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

아 요 부분은 고산하님 생각이 궁금했습니다!
대부분의 경우 생성자를 그냥 쓰게 되는데 따로 분리한 이유가 있는지 궁금해서 여쭤봤네요~
init함수를 따로 가져가는 이유는 spring의 클래스들은 특유의 lifecycle을 가지고있기 때문이라고 생각하는데요.
추후 복잡도가 높아질수록 초기화 시점을 따로 가져가는게 더 큰 이점으로 작용하게 됩니다.
굳이 고치실 필요는 없어요~

@changgunyee changgunyee merged commit 5037359 into next-step:headf1rst Aug 26, 2022
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.

2 participants