Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3주차 미션 / 서버 3조 - 임수빈 #2

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

limsubinn
Copy link

HttpRequest

  • InputStream으로 들어온 데이터를 넘겨 request 객체를 생성한다.
  • StartLine: 파싱 -> Method, Path, Version
  • Header: 파싱 -> (key, value)로 관리
  • Body: 헤더의 Content-Length값을 함께 넘겨주어 값을 읽는다.

HttpResponse

  • OutputStream 객체와 request로 들어온 Http version을 넘겨 response 객체를 생성한다.
  • forward(200 status code): Body, StartLine, Header를 쓰고 response
    • 헤더에 Content-Type, Content-Length 값을 쓴다.
  • redirect(302 status code): StartLine, Header를 쓰고 response
    • 헤더에 Location 값을 쓴다.
    • isLogin 여부를 함께 받아 true이면 헤더에 Set-Cookie 값을 쓴다.

Copy link
Contributor

@jung-woo-kim jung-woo-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요!

전체적으로 너무 잘짜주셔서 리뷰할 부분 찾느라 정말 힘들었네요,,😮😮
테스트도 좋고, enum활용, stream 등 동작 파라미터화 활용 모두 감탄하면서 봤어요!
정말 쥐어짜서 리뷰해드렸습니다!!

싱글톤으로 컨트롤러들을 관리하는 부분만 한번 고려해주세요👍
정말 잘하시고 계시는 것 같습니다!!
고생하셨어요!

.orElse(null);
validateMethod(m);
return m;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스트림 활용하신 부분 일단 너무 좋은 것 같아요!
enum의 valueOf 메서드가 똑같은 기능을 제공하는데 한번 알아보시겠어요??👍

.orElse(null);
validateHttpStatus(c);
return c;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스트림 장인이 여기 숨어계셨군요,,😮😮

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적인 Enum Class 활용 너무 좋습니다!
Enum에서 제공하는 values나 valueOf 같은 메서드들 한번 참고해보세요!🙂

public String toString() {
StringBuilder stringBuilder = new StringBuilder();
header.forEach((key, value) ->
stringBuilder.append(key.getHeader()).append(": ").append(value).append("\r\n"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헉 forEach와 람다의 활용 너무 깔끔하네요,,👍

controllers.put(RequestUrl.SIGNUP_URI.getUrl(), new SignupController());
controllers.put(RequestUrl.LOGIN_URI.getUrl(), new LoginController());
controllers.put(RequestUrl.LIST_URI.getUrl(), new ListController());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 구현할 시 사용자 요청이 엄청나게 많아지면 어떨까요?!
한번의 요청마다 Controller들을 엄청나게 생성하게 될 거에요!
Controller는 상태를 갖지 않기 때문에 프로그램 내에서 하나씩만 존재하는 것이 효율적입니다! (싱글톤)
즉, map() 메서드를 RequestHandler가 생성될때마다 계속 수행하지 말고, static{} 안에 contollers.put()들을 모아두는 것이 좋아보여요!

static은 객체 생성 시마다 실행되는 것이 아니고 클래스에 고정되어있기 때문에 한번만 실행돼요! 🧐

@limsubinn
Copy link
Author

리뷰 감사합니다!!
말씀해주신 부분 잘 고려해보겠습니다 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants