[웹서버 - step2] 효오 미션 제출합니다. #84
Conversation
| private final Map<HttpMethod, Controller> mapping = new HashMap<>(); | ||
|
|
||
| { | ||
| mapping.put(HttpMethod.GET, this::getMapping); | ||
| mapping.put(HttpMethod.POST, this::postMapping); | ||
| } | ||
|
|
||
| protected Optional<String> templateEngine(String filePath, Object object) throws IOException { |
There was a problem hiding this comment.
컨트롤러가 아닌 view 클래스를 추가해서 템플릿 엔진을 처리하도록 해보면 어떨까요?
그리고 핸들바뿐만 아니라 다른 템플릿 엔진으로 바꿀 수 있는 구조면 더 좋겠네요
| } | ||
|
|
||
| protected void checkLogin(HttpRequest request) { | ||
| if (!request.cookie().contains("logined=true")) { |
There was a problem hiding this comment.
- Cookie 클래스를 추가해보면 어떨까요?
- 쿠키 객체에서 key, value로 logined 같은 값을 처리할 수 있는 구조는 어떨까요?
There was a problem hiding this comment.
HttpCookie 클래스가 있군요?!!
그럼.. 로그인 체크하는 로직을 모든 클래스가 알 필요가 없어 보이는데 다른 곳에서 처리해야되지 않을까요?
| return FileIoUtils.loadFileFromClasspath(filePath) | ||
| .map(body -> HttpResponse.success(request, TEXT_HTML, body)) | ||
| .orElse(HttpResponse.INTERNAL_SERVER_ERROR); | ||
| return HttpResponse.successByFilePath(request, MimeType.TEXT_HTML, INDEX_PAGE_LOCATION); |
There was a problem hiding this comment.
코드가 많이 간결해졌네요!! 👍
MimeType.TEXT_HTML도 컨트롤러에서 설정할 필요는 없어 보이는데, 좀더 개선할 방법 없을까요?
There was a problem hiding this comment.
다른 MimeType을 사용하는 경우가 있을 것 같아서 해당 메소드를 만들었습니다.
하지만 모든 컨트롤러에서 2xx, 3xx 상태코드를 가지는 Response를 생성할 때 같은 MimeTye을 변경해서 컨트롤러에서 설정하지 않도록 수정하였습니다.
| String sessionId = sessionManager.setAttribute("loginUser", maybeUser.get()); | ||
|
|
||
| response.applySessionCookie(sessionId); | ||
| response.applyLoginCookie(true); |
There was a problem hiding this comment.
쿠키 설정하는 메서드와 logined를 설정하는 메서드를 각각 호출해야될 필요가 있을까요?
There was a problem hiding this comment.
쿠키를 설정하는 메서드와 logined 설정하는 메서드를 분리했는데
굳이 Controller에서 각각 호출할 필요는 없을 것 같아 피드백 내용을 반영하였습니다.
src/main/java/webserver/Router.java
Outdated
| @@ -16,13 +14,20 @@ | |||
|
|
|||
| private static final Controller INDEX_CONTROLLER = new IndexController(); | |||
| private static final Controller USER_CONTROLLER = new UserController(); | |||
| private static final Controller LOGIN_CONTROLLER = new LoginController(); | |||
There was a problem hiding this comment.
컨트롤러 상수들을 따로 둬야할 이유가 있을까요?
put 할 때 바로 생성자를 호출하면 되지 않을까요?
There was a problem hiding this comment.
Path가 늘어날 때마다, Controller를 생성하고 싶지 않아서 상수로 따로 두었지만,
컨트롤러를 상수로 따로 두지 않고 싱글톤 변경하였습니다.
| private static final Map<Boolean, String> LOGINED_VALUE = new HashMap<>(); | ||
|
|
||
| static { | ||
| LOGINED_VALUE.put(true, "true"); |
There was a problem hiding this comment.
true, false 텍스트를 맵으로 관리할 필요가 있을까요?
Map<String, Object> pair로 하면 어떨까요?
There was a problem hiding this comment.
boolean 값을 String으로 변환하려고 사용했지만,
Map을 사용하지 않고 구현할 수 있어서 해당 Map을 제거했습니다.
|
|
||
| @Test | ||
| @DisplayName("존재하지 않는 HTML 파일을 load 할 경우 빈값을 가져온다.") | ||
| void loadFileFromClasspath2() { |
There was a problem hiding this comment.
테스트 코드니 메소드명을 한글로 쓰면 좀더 작명하기 나을거에요
안녕하세요. 현구님
아직 많이 부족하지만 일단 pr 보내고 계속해서 고쳐나가겠습니다!