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단계] 하마드(이건회) 미션 제출합니다. #548

Merged
merged 11 commits into from
Sep 26, 2023

Conversation

rawfishthelgh
Copy link

깃장(git long) 안녕하세요.
하마드입니다.

저번에 주신 소중한 리뷰 확인하고 반영해봤습니다ㅎㅎ
말씀주신대로, Mapper 관련 클래스를 비즈니스 로직이 결합된 패키지와 분리시키면
관심사의 분리가 일어나 비즈니스 로직에 집중할 수 있는 패키지 구조가 될 수 있는 것 같네요.
좋은 지적 감사합니다~!

/view 가 존재하는 api도 모두 제거하고 통일시켜 봤습니다. 이번에도 좋은 리뷰 부탁해요~!

@rawfishthelgh rawfishthelgh changed the title [MVC 구현하기 - 2단계] 하마드(이건회) 미션 제출합니다. [MVC 구현하기 - 3단계] 하마드(이건회) 미션 제출합니다. Sep 23, 2023
Copy link

@gitchannn gitchannn left a comment

Choose a reason for hiding this comment

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

하마드 안녕하세요!

요구사항이 모두 반영되지 않아서 Request Changes 할게요

  • app 모듈에 있는 모든 컨트롤러를 어노테이션 기반 MVC로 변경한다.
  • Legacy MVC를 제거하고 나서 DispatcherServlet도 app 패키지가 아닌 mvc 패키지로 옮겨보자.

이 두 가지도 같이 구현해서 다시 요청 보내주세요!

String jsonString = objectMapper.writeValueAsString(model);
PrintWriter printWriter = response.getWriter();
response.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);
printWriter.write(jsonString);

Choose a reason for hiding this comment

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


잘 되는거 확인했습니다

@@ -20,11 +19,6 @@ public class ManualHandlerMapping implements HandlerMapping {
@Override
public void initialize() {
controllers.put("/", new ForwardController("/index.jsp"));

Choose a reason for hiding this comment

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

인덱스도 대체할 수 있지 않을까요??

Copy link

@gitchannn gitchannn left a comment

Choose a reason for hiding this comment

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

마드씨 수고했습니다
담 미션도 ㅍㅇㄸ이요!


@RequestMapping(value = "/", method = RequestMethod.GET)
public ModelAndView showPage(HttpServletRequest request, HttpServletResponse response) {
return new ModelAndView(new JspView("/index.jsp"));

Choose a reason for hiding this comment

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

printWriter.write(jsonString);
}

private String extractJsonString(Map<String, ?> model, ObjectMapper objectMapper) throws JsonProcessingException {

Choose a reason for hiding this comment

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

extract보다는 generate에 가까운 것 같지만!
이름 정도는 넘어가보겠습니다

@gitchannn gitchannn merged commit 896e811 into woowacourse:rawfishthelgh 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.

3 participants