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

Day/20170406 #7

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MinKyoungSoo
Copy link

No description provided.

yongjunjung added a commit to yongjunjung/web-application-server that referenced this pull request Apr 13, 2017

public HttpRequest(InputStream in) throws Exception {
BufferedReader br = new BufferedReader(new InputStreamReader(in, "UTF-8"));
getRequestLine(br);
Copy link
Member

Choose a reason for hiding this comment

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

오호. 리팩토링 잘 하셨네요. 고민의 흔적이 느껴집니다.

어제 라이브 코딩에서 했듯이 이 각각의 method를 새로운 객체로 분리하는 시도를 해보시면 또 다른 재미를 느끼실 겁니다.

@@ -44,6 +114,38 @@ private void response200Header(DataOutputStream dos, int lengthOfBodyContent) {
}
}

private void response302Header(DataOutputStream dos, String redirectUrl) {
Copy link
Member

Choose a reason for hiding this comment

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

응답 처리하는 부분에 많은 중복 코드가 있는데요.
이 부분을 HttpResponse를 만들어 제거하는 방법을 고민해 보면 좋겠어요.

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