Repository navigation
Conversation
There was a problem hiding this comment.
[칭찬] UserController에 매개변수를 넘겨주고 return 받는 DTO를 잘 만드신 것 같아요. 만약에 다른 URL이 추가되어 Controller가 더 추가된다면 다른 DTO를 더 추가해서 넘기고 받을 때 좋을 것 같아요.
| parseRequest(); | ||
| } | ||
|
|
||
| private void parseRequest() throws IOException { |
There was a problem hiding this comment.
[칭찬] Request를 parsing 할 때 보는 사람이 읽기 쉽게 잘 나누어 정리하신 것 같아요.
There was a problem hiding this comment.
[칭찬] 저번 주에 테스트에 대한 고민을 이야기해 주셨었는데 JUnit을 이용한 테스트도 꼼꼼히 잘 만들어주셨네요. 좋은 예시로 많이 참고하겠습니다.
There was a problem hiding this comment.
Mockito 프레임워크를 사용해서 BufferedReader를 Mock 객체로 만들어서 테스트를 작성했었는데 Mock으로는 원하는 방향으로 테스트할 수가 없어서 Mock객체 없이 단위테스트 처럼 테스트를 작성했습니다.
Mock에 대해서 궁금하시면 찾아보셔도 좋을 것 같습니다!
| HttpRequest request = new HttpRequest(br); | ||
| HttpResponse response = new HttpResponse(dos); | ||
|
|
||
| if(request.getRequestPath().startsWith("/user")){ |
There was a problem hiding this comment.
[질문] 웹 사이트의 url이 추가되면 RequestHandler의 로직이 점점 추가되고 길어질 것 같아서 url에 따라서 로직이 추가되고 하는 것을 한 곳에서 처리했으면 좋겠는데 저도 어떻게 하는게 잘하는 것일지 고민이 되어 같이 고민하면 좋을 것 같아서 일종의 메모를 남겨봅니다.
There was a problem hiding this comment.
스프링 프레임워크로 예를 들면 컨트롤러에서 받아 처리하는데, 하나의 컨트롤러에서 처리한다면 그것 또한 컨트롤러가 점점 복잡해져서 컨트롤러 앞에 프론트 컨트롤러를 두는 걸로 알고 있습니다.
요청 -> FrontController -> (예를들어 User도메인을 호출했다면) UserController
이런식으로요.
이렇게 되면 http요청이 들어오면 프론트 컨트롤러가 입구가 되는거죠. 거기서 도메인별로 또 나눠서 도메인에 맞는 컨트롤러가 호출되구요.
There was a problem hiding this comment.
괜찮으시다면 그렇게 호출되는 구조로 한 번 리팩토링하셔서 일요일에 예를 들어 설명해주시면 이해가 잘 될 것 같아요
| if("GET".equals(request.getMethod())){ | ||
| return createResponse200("./webapp/user/form.html"); | ||
| } | ||
| throw new IllegalStateException("HTTP 메서드를 지원하지 않습니다."); |
There was a problem hiding this comment.
[질문] Validation Check를 해서 IllegalStateException를 발생시키는 것은 매우 좋은 것 같습니다. 그런데 제가 잘 몰라서 그런 거일 수도 있는데 이 Exception을 받아서 처리하는 곳이 없어보이는데 이 exception이 발생하면 어떻게 되는지 궁금하네요.
There was a problem hiding this comment.
지금 상황에서 예외가 터지면 예외는 잡히지 않고 계속 밖으로 던져지게 됩니다.
아직은 따로 잡는곳이 없어서 사용자에게 오류가 보여집니다.
예외도 잡을 것이냐 던질 것이냐를 정해야하는데, 제가 생각하는 (배운) 예외의 좋은 처리 방식은 예외를 밖으로 던져서 애플리케이션 상위에서 공통으로 잡는 것이라고 배웠습니다.
물론 100% 이 방법이 맞는 것은 아니겠지만, 메서드 실행부분에서 예외를 catch로 잡아버린다면, 나중에 애플리케이션이 커졌을 때 어디서 문제인지 파악하기 힘들다고 생각합니다.
상황에 따라서 다르겠지만 저는 체크예외가 터진다면, 그때는 체크예외를 잡고 언체크예외 (런타임 예외) 로 바꿔서 던지고 애플리케이션 상위에서 공통으로 처리하는 편입니다.
There was a problem hiding this comment.
아 좋은 방법이네요. 저도rest api를 구현한다면 웹 페이지 사용자에게 적절한 http status code와 함께 돌려주면 좋을 것 같다고 생각했는데 상위에서 한 번에 exception을 catch해서 던져주는 로직이 있으면 좋을 것 같네요. 괜찮으시다면 이것도 한 번 간단하게 구현해주셔서 보여주실 수 있을까요??
There was a problem hiding this comment.
[칭찬] 그렇네요
"메서드 실행부분에서 예외를 catch로 잡아버린다면, 나중에 애플리케이션이 커졌을 때 어디서 문제인지 파악하기 힘들다고 생각합니다."
예외처리하는 위치에 따라 문제 파악을 손쉽게 할 수 있겠네요!
|
|
||
| public HttpRequest(InputStream in) throws IOException{ | ||
| reader = new BufferedReader(new InputStreamReader(in)); | ||
| public HttpRequest(BufferedReader reader) throws IOException{ |
There was a problem hiding this comment.
[질문] InputStream에서 BufferedReader로 생성자 파라미터를 수정한 이유가 있나요??
어떤 이유인지 궁금합니다
| "/user/create", this::saveMember | ||
| ); | ||
|
|
||
| public ResponseDto run(RequestDto request) { |
There was a problem hiding this comment.
[질문] DTO 객체를 반환해주는 함수에 RUN이라는 네이밍이 저는 낯설게 느껴지네요!
어떤 이유로 RUN을 사용했는지 궁금해요!
| private String method; | ||
| private String url; | ||
| private String version; | ||
| private String requestPath; |
There was a problem hiding this comment.
[칭찬] url과 requestPath를 둘다 넣어놓은게 좋습니다!
url과 requestPath 의 차이점을 알아보니, 일반적으로 url은 전체 URL을 나타내고, requestPath 는 URL의 경로부분만 표현한다네요!
| if("GET".equals(request.getMethod())){ | ||
| return createResponse200("./webapp/user/form.html"); | ||
| } | ||
| throw new IllegalStateException("HTTP 메서드를 지원하지 않습니다."); |
There was a problem hiding this comment.
[칭찬] 그렇네요
"메서드 실행부분에서 예외를 catch로 잡아버린다면, 나중에 애플리케이션이 커졌을 때 어디서 문제인지 파악하기 힘들다고 생각합니다."
예외처리하는 위치에 따라 문제 파악을 손쉽게 할 수 있겠네요!
3 participants
No description provided.