Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MVC 구현하기 - 3단계] 도기(김동호) 미션 제출합니다. #603

Merged
merged 16 commits into from
Sep 26, 2023

Conversation

kdkdhoho
Copy link

@kdkdhoho kdkdhoho commented Sep 25, 2023

안녕하세요 두둠! 어느새 MVC 미션도 마지막이네요!
그럼 이번에도 잘 부탁드립니다 🫡

공유 사항

  1. asis 패키지에 있는 ForwardController를 제거하라는 요구사항이 있었는데요, 이 컨트롤러가 하는 역할처럼 "/' 로 들어오는 요청을 무조건 index 페이지로 응답하는 DefaultController가 디스패처 서블릿 내부에 있어도 괜찮겠다 판단해서, 제거하는 겸 mvc 패키지로 옮겼습니다!
  2. 미션 힌트에 model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다. 라고 되어 있는데, 이게 무슨 말인지 이해가 안가네요 🧐

@kdkdhoho kdkdhoho self-assigned this Sep 25, 2023
Copy link

@younghoondoodoom younghoondoodoom left a comment

Choose a reason for hiding this comment

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

안녕하세요 도기🙌 많이 고민하신게 느껴지는 코드였던 것 같아요! 정말정말 수고하셨습니다👍

미션 힌트에 model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다. 라고 되어 있는데, 이게 무슨 말인지 이해가 안가네요 🧐

제 생각은 다음과 같습니다!

  1. 데이터가 1개면 값을 그대로 반환한다.
{
    "nickname" : "도기",
    "group" : "백엔드"
}
  1. 2개 이상이면 map 형태 그대로 json으로 반환한다.
{
    "user" : {
        "nickname" : "도기",
        "group" : "백엔드"
    }, 
    "computer" : {
        "model" : "맥북"
    }
}

제가 잘못 이해했을 수도 있으니까 정확한 답을 찾아내시면 공유 부탁드립니다..!
파이팅하세요💪

Choose a reason for hiding this comment

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

저는 다양한 View를 사용하기위해서 ViewResolver를 만들고, support 메소드를 통해 확장자를 확인했어요! ViewAdaptor를 통해서 확인해도 되겠지만, 다른 View를 사용하려면 코드가 직접 수정되어야한다는 단점이 존재하는 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신 대로 Adapter의 결과를 ModelAndView로 어떻게 변환할까 정말 많은 고민과 시도를 해보았는데요.
결론부터 말씀드리면, 일단 이 구조를 유지해도 괜찮을지 여쭤보고 싶습니다.

현재 코드 구조상 HandlerExecution에서 요청에 해당하는 메서드를 처리한 뒤, 그 결과가 무엇이든 ModelAndView 타입으로 변환시켜줘야 한다고 판단했습니다.
그래서 HandlerExecution가 필드로 ViewResolver를 가져야 하는데 이러면 매번 생성되는 HandlerExecution마다 새로운 ViewResolver 객체를 만들어 넣어줘야 하는 점이 매우 맘에 안들었습니다.

이를 개선하려면, DispatcherServlet만이 ViewResolver를 의존하고, HandlerAdapter의 결과로 받은 Object 타입의 결과를 viewResolver를 통해 적절히 ModelAndView를 만들거나 거기서 바로 처리하도록 하면 좋을 것 같다는 생각을 했어요.

이를 구현하려면 구조나 객체들의 역할을 전반적으로 변경해야 할 텐데 이는 시간상 불가능하다고 판단했습니다.

차선책으로 HandlerExecutionHandlerAdapter에서 ModelAndView로 변환하는 작업을 처리한다고 했을 때, 두둠이 말씀하신 방법을 적용할 수 있겠지만 ViewResolve(ViewAdapter)가 가질 JspView 객체를 생성할 때, JspView가 필드를 가지기에 불가능했습니다. 두둠은 ModelAndView를 살짝 변형하셔서 적용하신 것 같은데, 제 기준에서는 ModelAndView를 최대한 수정하지 않고 구현하고 싶습니다 😢

Choose a reason for hiding this comment

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

그렇군요! 저도 이 부분 때문에 정말 고민이 많았는데요, 이미 보신것 같긴한데 저는 ModelAndView가 가지고있는 View를 Object Type으로 변경하고, Dispatcher Servlet에서 ViewResolver를 통해 View Type의 객체를 만들도록 했어요. 저도 레거시 코드인 ModelAndView를 수정하는게 조금 찝찝했습니다🥲 이 구조도 충분히 괜찮은 구조라고 생각합니다!

Comment on lines +19 to +21
if (viewName.startsWith(REDIRECT_PREFIX)) {
return JSP;
}

Choose a reason for hiding this comment

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

다른 view가 생긴다면 코드가 직접 수정되어야하지 않을까요?

Copy link

@younghoondoodoom younghoondoodoom left a comment

Choose a reason for hiding this comment

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

안녕하세요 도기! 이번 미션에서 생각할 부분에 대해서 충분히 고민하신것 같아요! 이만 머지 하겠습니다.
다음 미션도 파이팅 하세요💪

@younghoondoodoom younghoondoodoom merged commit de57c58 into woowacourse:kdkdhoho Sep 26, 2023
1 check failed
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