[MVC 구현하기 - 3단계] 오찌(오지훈) 미션 제출합니다 #255
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
Gyuchool
suggested changes
Sep 27, 2022
README.md
Outdated
| ## 3단계 - JSON View 구현하기 | ||
|
|
||
| - [x] 힌트에서 제공한 UserController 컨트롤러가 json 형태로 응답을 반환한다. | ||
| - [ ] 레거시 코드를 삭제하고 서버를 띄워도 정상 동작한다. |
| import nextstep.mvc.view.ModelAndView; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class UserControllerTest { |
| } | ||
|
|
||
| private Object getAttributes(final Map<String, ?> model) { | ||
| if (model.size() > 1) { |
|
파일 체인지에 없어서 따로 코멘트 남길게요!
이 부분에 대해서 저도 리뷰를 받고 해결하다가 앞선 리뷰에서 얘기했던 MappingRegistry에서 Optional을 반환하는 이유를 찾은 것 같아서 공유해드려요!
이 부분은 저도 놓쳤는데😅 위와 비슷한 방식으로 처리하면 되지않을까 싶네요 저도 구현해보고 말씀드릴게요! |
Gyuchool
approved these changes
Sep 29, 2022
| @RequestMapping(value = "/api/user", method = RequestMethod.GET) | ||
| public ModelAndView show(HttpServletRequest request, HttpServletResponse response) { | ||
| final String account = request.getParameter("account"); | ||
| if (account == null) { |
There was a problem hiding this comment.
Suggested change
| if (account == null) { | |
| if (Objects.isNull(account)) { |
위에처럼 Objects의 메서드를 사용하지 않으신 이유가 있을까요?
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.
안녕하세요 기론 😄 3단계를 빨리 마무리하고 리뷰 요청 드립니다!
새로 추가되는 UserController 메서드도 테스트 하기 위해 InMemoryUserRepository를 UserRepository로 한 단계 추상화했습니다!
사실 나머지 Controller들도 테스트를 추가해야 할지가 조금 고민이네요...!
또다른 고민이 있다면 서블릿에 매핑되지 않은 URL의 경우 어떻게 해야 404.jsp를 보내줄 수 있을지, ServletException이 발생했을 때 500.jsp로 리다이렉션해주는 것은 어떨지에 대한 고민입니다. 에러에 관련된 jsp 파일을 리다이렉션 하는 부분에서 좋은 생각이 있으시다면 공유 부탁드려요 ㅎㅎ :)
3단계도 잘 부탁드립니다!