3주차 미션 / 서버 2조 임제형 #3

kisusu115 · 2024-10-03T13:07:57Z

No description provided.

RequestStartLine 클래스 선언하여 startLine 분리, URLDecoder 사용하여 urlencoded 된 문자열 decode, HttpResponse에서 StringBuilder 사용하여 header 모아서 전송

RequestStartLine 객체 생성 책임을 RequestStartLine 클래스에게로 변경

hamhyeongju

코드 잘 봤습니다! 전반적으로 미션 의도에 맞게 코드를 잘 작성해 주신 것 같아서 코드 리뷰드리기 수월했네요. 1주 차에 언급했던 객체 지향에 관한 내용은 10주 차까지 쭉 이어집니다! 잠깐 배우고 넘어가는 내용이 아니라 백엔드 개발에 기반이 되는 부분이라 이 부분 잊지 않고 신경쓰시면서 미션 진행해 주시면 더 좋을 것 같습니다!
코드 리뷰 확인하시고 업로드된 구현 코드와 비교해 보시면 좋을 것 같습니다! 수고 많으셨습니다!

hamhyeongju · 2024-10-05T07:28:48Z

src/main/java/controller/ListController.java

+
+        if (cookie != null && cookie.contains("logined=true")) {
+            String location = USER_LIST_HTML.getRoute();
+            httpResponse.redirect(location);


정상적인 경로에 대한 요청을 응답할 경우, redirect 보단 forward를 사용하면 좋을 것 같습니다. redirect 는 응답 후 get으로 재요청을 명령하기 때문에 네트워크 통신하는데 리소스가 더 소모됩니다!

hamhyeongju · 2024-10-05T07:35:42Z

src/main/java/controller/LoginController.java

+
+        if(isLoginSuccessed){
+            String location = INDEX_HTML.getRoute();
+            httpResponse.redirectSettingCookie(location);


early return 을 사용해 보는건 어떨까요?

hamhyeongju · 2024-10-05T07:36:19Z

src/main/java/controller/LoginController.java

+
+    private boolean checkIdPwValid(String userId, String password){
+        User user = repository.findUserById(userId);
+        if(user != null && user.getPassword().equals(password)) return true;


조건문 없이 `user != null && user.getPassword().equals(password) 만 리턴해도 결과는 같을 거 같네요.

hamhyeongju · 2024-10-05T07:38:02Z

src/main/java/db/MemoryUserRepository.java

@@ -23,6 +23,7 @@ public static MemoryUserRepository getInstance() {

    public void addUser(User user) {
        users.put(user.getUserId(), user);
+        System.out.println(user.getUserId()+"님 이 회원가입 하였습니다.");


실무에선 System.out을 사용하지 않습니다! 지금부터라도 log 사용에 익숙해 지시는게 좋을 것 같습니다!

hamhyeongju · 2024-10-05T07:42:05Z

src/main/java/enums/http/header/EntityHeader.java

@@ -0,0 +1,20 @@
+package enums.http.header;
+
+public enum EntityHeader {


음... 이건 제 생각이지만, HttpHeader 이넘클래스 하나로 상수를 관리하는게 좋지 않을까요? 어떤 헤더가 EntityHeader에 포함되는지 일일히 기억하기 쉽지 않을 것 같습니다..!

hamhyeongju · 2024-10-05T07:52:49Z

src/main/java/http/HttpResponse.java

+    private void sendBody(byte[] body) {
+        try {
+            dos.write(body, 0, body.length);
+            dos.flush();


IOStream 자원을 사용했다면 항상 마지막에 명시적으로 close 해줘야합니다!

hamhyeongju · 2024-10-05T07:55:52Z

src/main/java/http/HttpResponse.java

+            dos.flush();
+        }
+        catch (IOException e) {
+            log.log(Level.SEVERE, e.getMessage());


sendHeader, sendBody 에서 try-catch 문을 통해 IOException을 잡아서 처리해주고 있는데, 어차피 forward 함수에서 해당 예외를 throw 해주고 있고, 클라이언트 코드 자체에서도 마찬가지로 try-catch 문으로 Exception을 처리해주고 있기 때문에 특별한 처리가 필요하지 않다면, 그냥 throw하는게 코드 응집도가 높아질 것 같아요! 같은 맥락의 예외 처리가 프로젝트에 전파되어 있습니다!

hamhyeongju · 2024-10-05T07:59:31Z

src/main/java/webserver/RequestHandler.java

 public class RequestHandler implements Runnable{
    Socket connection;
    private static final Logger log = Logger.getLogger(RequestHandler.class.getName());

+    private final Repository repository;
+    private Controller controller = new ForwardController();


Repository가 해당 클래스에선 안쓰이는 것 같은데, 의존성을 제거해주면 좋을 것 같아요.

hamhyeongju · 2024-10-05T08:02:34Z

src/main/java/webserver/RequestHandler.java

        } catch (IOException e) {
-            log.log(Level.SEVERE, e.getMessage());
+            // 예외 처리할 때 400 응답 뱉어야 될 것 같은데.. 헷갈리네


자세한 예외처리는 다음에 자세히 배웁니다! ㅎㅎ HTTP 에 대해 잘 이해하신 것 같습니다! 좋은 고민인 것 같네요!

hamhyeongju · 2024-10-05T08:03:58Z

src/main/java/webserver/RequestMapper.java

+            return new RequestMapper(controller, httpRequest, httpResponse);
+        }
+
+        Map <String,Controller> controllers = setControllerMap();


참고로 static 영역에서 controllers를 관리하면 멀티쓰레드 환경에서 인스턴스를 공유할 수 있어서 자원을 보다 효율적으로 활용할 수 있습니다! 물론 그래서 발생할 수도 있는 문제가 있는데 일단 우리 코드에서 문제되는 부분은 없어서 static으로 관리해도 괜찮을 것 같습니다.
이 부분은 싱글톤을 키워드로 검색해보시는걸 추천드립니다!

항상 친절한 답변 감사합니다!
말씀 해주신 부분들 다시 한번 생각해보겠습니다
3주차 까지 너무 감사했습니다!!!

kisusu115 added 5 commits October 1, 2024 23:18

feat: 1차 코드 구현

7d43eb0

feat: RequestMapper 구현

fa4ebd1

refactor: HttpRequest 정적 팩토리 메소드 from()에서의 메소드 추출 및 필드/ 메소드 이름 변경

d2f8cea

refactor:

4c2e451

RequestStartLine 클래스 선언하여 startLine 분리, URLDecoder 사용하여 urlencoded 된 문자열 decode, HttpResponse에서 StringBuilder 사용하여 header 모아서 전송

refactor:

4979c3a

RequestStartLine 객체 생성 책임을 RequestStartLine 클래스에게로 변경

hamhyeongju reviewed Oct 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3주차 미션 / 서버 2조 임제형 #3

3주차 미션 / 서버 2조 임제형 #3

kisusu115 commented Oct 3, 2024

hamhyeongju left a comment

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

hamhyeongju Oct 5, 2024

kisusu115 Oct 5, 2024

		@@ -0,0 +1,20 @@
		package enums.http.header;

		public enum EntityHeader {

3주차 미션 / 서버 2조 임제형 #3

Are you sure you want to change the base?

3주차 미션 / 서버 2조 임제형 #3

Conversation

kisusu115 commented Oct 3, 2024

hamhyeongju left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment