[MVC 구현하기 - 3단계] 디우(김동규) 미션 제출합니다. #289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
her0807
suggested changes
Sep 27, 2022
Comment on lines
28
to
30
| final Object attribute = getAttribute(model); | ||
| return objectMapper.writeValueAsString(attribute); | ||
| } |
There was a problem hiding this comment.
안녕하세요 디우!
objectMapper 사용 방법을 새롭게 익히느라 힘드셨을 것 같아요.
해당 로직은 데이터가 하나일 경우에 데이터 값 그대로 반환해주려고 작성하신 것으로 보이는데
value의 개수를 아는 상태에서 스트림을 사용해서 데이터를 가져오는 로직이 조금 무거워보여요!
아래와 같은 로직으로 스트림 없이 추출할 수도 있을 것 같아요
Suggested change
| final Object attribute = getAttribute(model); | |
| return objectMapper.writeValueAsString(attribute); | |
| } | |
| if (model.size() == 1) { | |
| final String key = (String) model.keySet().toArray()[0]; | |
| final Object value = model.get(key); | |
| objectMapper.writeValue(writer, value); | |
| return; | |
| } |
There was a problem hiding this comment.
오호..!! 그러네요 이미 size를 아는데, 굳이 스트림을 돌면서 가져오는건 낭비였네요..😅
추가적으로 코드 개선해보았습니다!!
또한 writerWithDefaultPrettyPrinter() 부분도 제거해보았는데, 저희 팀의 경우 프런트와 실제로 네트워크를 타고 통신할 때 공백을 추가해주지 않는다고 하더라구요..불필요한 비용이 추가되는 것 같아서 제거해보았습니다!!
| return model.values() | ||
| .stream() | ||
| .findFirst() | ||
| .orElseThrow(); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 수달!!
오늘까지 PR 마지막 제출이라서 빠르게 3단계 구현해서 제출합니다..!!
이번 단계에서는 JsonView 가 제대로 동작하는지 테스트하는 것이 까다로웠었습니다..
저는 참고한 글 에서
How to test a method using a PrintWriter를 참고해서 테스트를 작성하였습니다ㅎ.ㅎ또한 힌트에서
model에 데이터가 1개면 값을 그대로 반환하고 2개 이상이면 Map 형태 그대로 JSON으로 변환해서 반환한다.라는 말이 있어 해당 요구사항에 맞게 구현해보았습니다!!그리고 이번에도 역시 마찬가지로 하나하나씩 Controller 어노테이션 기반으로 갈아끼우는 작업을 하고 테스트 코드도 추가해보았는데요..!! 예외 케이스들까지 모두 고려하면 테스트가 너무 많아질 것 같아..우선 보이는 정상 기능들에 대해서만 추가하였습니다! (시간 나는대로 예외 케이스들도 다룰 수 있도록 추가해보겠습니다..ㅠ.ㅠ)
모든 컨틀롤러에 대해서 리팩터링을 진행한 후
ForwardController제거,ManualHandlerMapping제거 순으로 차근차근 asis 패키지를 제거해주었습니다! (요구사항에asis 패키지에 있는 레거시 코드를 삭제해도 서비스가 정상 동작하도록 리팩터링하자.라고 되어있어서 모두 제거해주었는데,Controller인터페이스나ControllerHandlerAdapter는 나둬도 backward 호환성은 제공할 수 있어 좋을 것 같다는 생각이 들었는데 그래도 요구사항에 맞게끔 구현해주었습니다!구현 기능 목록(구현 내용)
1. JspView 클래스를 구현한다.
이 부분은 2단계 미션 진행하면서
DispatcherServlet에서 ModelAndView 를 이용해 JspView 를 렌더링하도록 책임을 분리하였기 때문에 추가적인 작업은 진행하지 않았습니다 😃2. JsonView 클래스를 구현한다.
ModelAndView 에서 객체를 받았을 때 ObjectMapper 를 이용해서 JSON 형태로 반환해줄 수 있도록 구현하였습니다. 추가적으로 model 에 담긴 데이터가 한 개인 경우에는 그대로 반환해줄 수 있게 분기처리를 해주었습니다.
UserController가 제대로 동작하는지 확인하기 위해서 User 도메인에 대해서 Getter 를 열어주었습니다..!!그런데 JsonView 를 단위 테스트하는게 어려웠었는데요..앞서 언급드린 글을 참고해서 stringWriter 의 toString() 을 이용해 원하는 형식으로 출력되는지 확인하는 방식으로 테스트를 진행해주었습니다.
3. Legacy MVC 제거하기
컨트롤러 하나씩 리팩터링을 진행하며 테스트 코드를 추가해 기존의 동작을 리팩터링 했을 때도 잘 동작하는지 확인할 수 있도록 해주었습니다. 또한 asis 패키지의 클래스들도 한 번에 제거하는 것이 아니라 하나씩 제거해 나가는 방식으로 리팩터링을 진행하였습니다!!
체크 리스트
마지막 단계도 잘 부탁드립니다!!