3주차 미션 / 서버 1조 김지현 #6
Conversation
|
일단 질문 주신 부분만 확인해봤는데, 쿠키 값 확인할 때 null 처리를 안 해주셔서 NullPointException 이 발생했고, 조건문 블럭이 실행이 안되는 거였네요! 새벽에 미션 하시느라 고생 많으셨습니다! 코드 리뷰도 곧 달아드릴게요! |
| public class HomeController implements Controller{ | ||
| @Override | ||
| public void execute(Request request, Response response) throws IOException { | ||
| response.redirect("/index.html"); |
There was a problem hiding this comment.
정상적인 경로에 대한 응답은 redirect 보단 forward로 처리해주세요!
리다이렉트는 재요청을 명령하는 응답을 내리기 때문에 불필요하게 한 번 더 네트워크 통신이 발생해요. 네트워크 통신은 생각보다 비싸고, 리소스가 많이 소모되기 때문에 리다이렉트의 목적을 이해하고 필요한 곳에만 사용해야 합니다!
서버의 입장에서 서버 성능 개선에 가장 중요한 부분이 불필요한 외부 통신을 줄이는 것이란걸 알고 계시면 좋을 것 같습니다.
| } | ||
|
|
||
| public void addCookie(String cookie) throws IOException { | ||
| dos.writeBytes( HttpHeader.HTTP_302.getHeaderValue() + "\r\n"); |
There was a problem hiding this comment.
쿠키를 추가하는 부분이랑 302 응답 코드를 작성하는 부분이 왜 같이 있는지 모르겠네요 👀
메서드가 하나의 일을 하고 있지 않은 것 같습니다!
| } | ||
| } | ||
|
|
||
| public void addCookie(String cookie) throws IOException { |
There was a problem hiding this comment.
쿠키는 헤더의 일부죠! addCookie 보단 addHeader 메서드를 통해 HttpHeader와 value 값을 받아 사용하면 범용적으로 쓸 수 있는 메서드가 될 것 같습니다!
|
|
||
| User user = MemoryUserRepository.getInstance().findUserById(userId); //findUser 여기서 user 받기 | ||
|
|
||
| if(user.getPassword().equals(password)) { |
There was a problem hiding this comment.
여기서도 NullPointException이 발생할 수 있을 것 같지 않나요??
| if(user.getPassword().equals(password)) { | ||
| response.addCookie("logined=true"); | ||
| response.redirect("/index.html"); | ||
| }else{ |
There was a problem hiding this comment.
1주 차 강의에서 else 예약어 사용을 지양하자고 언급했었죠. 1주 차 내용은 10주 차까지는 물론이고 , kuit 서버 파트를 넘어 백엔드 개발의 기반이 되는 내용이기 때문에 이 부분 계속 상기하면서 미션 진행하시면 좋을 것 같습니다!
| COOKIE("Cookie"), | ||
| CONTENT_TYPE("Content-Type"), | ||
| CONNECTION("Connection"), | ||
| HTTP_302("HTTP/1.1 302 Found"); |
|
|
||
| public class Request { | ||
| private String method; | ||
| private String path; |
There was a problem hiding this comment.
startLine에서 version 이 빠진 것 같죠? 위 두 필드와 http version까지 합쳐서 HttpRequestStartLine 클래스를 만들어 활용하면 좋을 것 같습니다!
| controllerMap.put(PathName.SIGNUP.getPathName(), new SignUpController()); | ||
| controllerMap.put(PathName.LOGIN.getPathName(), new LoginController()); | ||
| controllerMap.put(PathName.USER_LIST.getPathName(), new UserListController()); | ||
| controllerMap.put(PathName.HOME.getPathName(), new HomeController()); |
There was a problem hiding this comment.
이 코드들을 RequestMapper로 분리하는 것까지가 미션 요구사항이었는데 아쉽습니다 ㅜㅜ
| public class Response { | ||
| private static final String WEB_ROOT = "webapp"; | ||
| private final DataOutputStream dos; | ||
| private static final Logger log = Logger.getLogger(RequestHandler.class.getName()); |
There was a problem hiding this comment.
Response 클래스도 Request와 마찬가지로 Http 응답 메시지에 필요한 속성들을 필드로 가지고 있으면 아래 코드들을 더 가독성 좋게 쓸 수 있지 않았을까요?
쿠키 확인하고 userlist 조회하는 부분이 안 될때가 있는 것 같습니다. 피드백 부탁드려요!
항상 감사합니다:)