[톰캣 구현하기 - 3,4단계] 글렌(전석진) 미션 제출합니다. #401
Conversation
- 테스트 시간이 600ms 더 늘어나는 이슈로 제거합니다.
There was a problem hiding this comment.
글렌 안녕하세요! step 1,2때 이미 step3가많이 되어있어서인지 코드 수정이 많이 없었네요..!
솔직히 말씀드리면 글렌코드를 보면서 많이 배웠고 여기서 더 수정이 필요하다고 생각되는 부분을 찾지 못하였네요..ㅠㅠ 궁금한 부분에 대해서 적어보았습니다!
또한 패키기 구조에 대해서 고민이 있으셨다고 하셨는데 저는 이번 미션을 하면서 sessionManager에 의한 catalina패키지와 coyote 패키지 간의 패키지 순환참조가 발생하여 이부분에 대해서 고민이 있었습니다.
해결방안은 찾지 못하였는데 이부분에 대해 글렌의 의견이 궁금하네요!
고생 많으셨습니다!
|
|
||
| private static final Manager manager = new SessionManager(); | ||
| private static final Manager MANAGER = new SessionManager(); | ||
|
|
There was a problem hiding this comment.
SETTIONS는 소문자로 바꾸고 manager은 대문자로 수정하였는데 기준이 뭐였는지 물어봐도 될까요!?
There was a problem hiding this comment.
https://google.github.io/styleguide/javaguide.html#s5.2.4-constant-names
다음과 같은 링크에 상수에 관한 컨벤션이 나와있습니다!
지금 생각해보면 Manager 객체는 manager와 같이 소문자로 변수명을 작성해야 할 것 같습니다. 😂
생각을 나름 정리하자면...
static으로 선언된 상수라 하면, 어플리케이션이 실행되고, 끝나기 전까지 변할 가능성이 전혀 없어야 상수라고 할 것 같습니다.
하지만 Manager 인터페이스는 add(), remove() 메소드가 존재합니다.
해당 메소드는 Manager 객체의 상태를 변경시키는 메소드이므로, 해당 객체가 완전한 상수가 아닌 변수로 판단하여 manager라는 변수명이 더 명확할 것 같네요!
|
|
||
| private final List<HttpMethod> allowedMethod; | ||
| private final Collection<HttpMethod> allowedMethod; | ||
|
|
||
| public MethodNotAllowedException(List<HttpMethod> allowedMethod) { | ||
| public MethodNotAllowedException(Collection<HttpMethod> allowedMethod) { | ||
| super(); |
There was a problem hiding this comment.
collection으로 수정하므로써 더 유연한 코드가 된거같네요!
|
|
||
| public class ExceptionHandler { | ||
|
|
||
| private final Controller methodNotAllowedController; | ||
| private final Controller notFoundController; | ||
|
|
There was a problem hiding this comment.
필드가 많지 않은데 빌더패턴을 사용하신 이유가 따로 있을까요!? 순수 궁금!
There was a problem hiding this comment.
ExceptionHandler의 생성자의 파라미터 타입은 전부 Controller 인터페이스 입니다.
따라서 순서에 따라 MethodNotAllowController, NotFoundController가 바뀔 수 있습니다!
이러한 실수를 방지하기 위해 빌더 패턴을 사용하여 명시적으로 순서를 보장하였습니다!
| public class RegisterController extends HttpMethodController { | ||
|
|
||
| @Override | ||
| public HttpResponse handle(HttpRequest request) throws IOException { | ||
| HttpMethod httpMethod = request.getHttpMethod(); | ||
| if (httpMethod == HttpMethod.GET) { | ||
| return doGet(); | ||
| } | ||
| if (httpMethod == HttpMethod.POST) { | ||
| return doPost(request); | ||
| } | ||
| throw new MethodNotAllowedException(List.of(HttpMethod.GET, HttpMethod.POST)); | ||
| public RegisterController() { | ||
| handlers.put(HttpMethod.GET, this::doGet); | ||
| handlers.put(HttpMethod.POST, this::doPost); |
|
Kudos, SonarCloud Quality Gate passed!
|
|
글렌 안녕하세요. |
안녕하세요 채채!
3, 4단계 완료하고 미션 제출합니다!
이전 1, 2단계에서 3단계 요구사항인 리팩터링을 해놓은 상태라, 빠르게 구현할 수 있었네요!
4단계 요구사항인 스레드풀은
Connector클래스에ExecutorService필드로 구현하였습니다!다만 걸리는 것은 패키지 구조가 올바른가에 대해서인데..
이 부분은 같이 논의했으면 좋겠네요!