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, 4단계] 가비(이준희) 미션 제출합니다. #434

Merged
merged 29 commits into from
Sep 11, 2023

Conversation

iamjooon2
Copy link
Member

@iamjooon2 iamjooon2 commented Sep 9, 2023

안녕하세요 호이!
3, 4단계 리뷰요청 드립니다!

처음에는 톰캣 코드를 보며 catalina와 coyote는 무슨 역할을 할까 많이 찾아보았는데요
제 생각대로 만들어보고, 나중에 톰캣과 비교하는게 더 의미있는 미션이 아닐까 싶어서
제 마음대로 만들어보았습니다!..
어색한 부분이 있다면.. 따끔한 피드백 부탁드리겠습니다

감사합니다🙇🏻‍♂️

@iamjooon2 iamjooon2 self-assigned this Sep 9, 2023
@sonarcloud
Copy link

sonarcloud bot commented Sep 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Copy link

@This2sho This2sho left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 가비!
미션한다고 리뷰가 많이 늦었네요 😅

3, 4단계 리팩토링한 거 보고 많이 배웠습니다.
코드가 엄청 깔끔해졌네요 👍

리팩토링 잘해주셔서 딱히 리뷰 남길게 없었지만..
동시성 관련해서 커맨트 남긴 부분 한번 확인해주시면 감사하겠습니다!
(수정할 부분은 없는 거 같아서 바로 머지하겠습니다 고생하셨습니다 🙇‍♂️)

}
doDefault(request, response);
}

Choose a reason for hiding this comment

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

오홍 isMethod도 깔끔하고 doDefault404 페이지로 보내는거 너무 좋은데요~? 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

지금 다시 보니 doDefault() 메서드 접근 제어자를 private으로 해둬서 상속받은 하위객체들에서는 적용이 안되겠네요...ㅋㅋㅋㅋㅋ

URIS.put("/login", new LoginController());
URIS.put("/register", new RegisterController());
URIS.put("/index", new IndexController());
}

Choose a reason for hiding this comment

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

초기화 블럭 👍

response
.statusLine(statusLine)
.contentType(HTML.getValue())
.contentLength(responseBody.getValue().getBytes().length)

Choose a reason for hiding this comment

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

responseBody.getValue().getBytes().length 이 부분 중복이 많이 생기는데
ResponseBody에 메소드로 만들어도 괜찮을거 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

아이고 이부분을 놓쳤었네요!


@Override
protected void doGet(HttpRequest request, HttpResponse response) {
ResponseBody responseBody = new ResponseBody(FileReader.read(RESOURCE));

Choose a reason for hiding this comment

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

FileReader 좋은데요~!? 👍

}

public Connector(final int port, final int acceptCount) {
public Connector(final int port, final int acceptCount, final int maxThreads) {
this.threadPoolExecutor = (ThreadPoolExecutor) Executors.newFixedThreadPool(maxThreads);
this.serverSocket = createServerSocket(port, acceptCount);

Choose a reason for hiding this comment

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

newFixedThreadPool로 생성하면 큐의 최대 사이즈는 몇개가 될까요?!
createServerSocket에서 acceptCount는 어떤 옵션일까요?!

최대 ThradPool의 크기는 250, 모든 Thread가 사용 중인(Busy) 상태이면 100명까지 대기 상태로 만들려면 어떻게 할까요?!

Choose a reason for hiding this comment

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

죄송합니다. 질문이 많네요 ㅋㅋ;;
가비가 코드를 깔끔하게 잘 짜주셔서 리뷰 남길게 별로 없어서..

Copy link
Member Author

Choose a reason for hiding this comment

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

image

  1. 큐의 사이즈는 무한대인것 같습니다..?(Integer.MAX_VALUE)

image
2. 타고 타고 들어가니까 ServerSocket의 backlog 인자네요. 큐에 들어오는 커넥션 수였습니다. 기본값은 50!

  1. acceptCount를 100으로, maxTrhead를 250으로 Connector를 생성해주면 되겠군요...! (좀 헷갈리네요!)

칭찬과 함꼐 꼼꼼히 질문 남겨주셔서 감사합니당😋


private static final int KEY_INDEX = 0;
private static final int VALUE_INDEX = 1;
public class HttpRequest {

Choose a reason for hiding this comment

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

👍 깔꼼!


public class ResponseViewer {
private static final String ENTER = "\r\n";
private static final String BLANK = " ";

Choose a reason for hiding this comment

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

별건 아니지만 저도 이번에 알게된건데 공유합니다!
SP(공백), CRLF(엔터) <= (김영한님 네트워크 강의에서 이런걸 쓰시더라구요!)

@This2sho This2sho merged commit 95c43ab into woowacourse:iamjooon2 Sep 11, 2023
2 checks passed
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