[1단계 - Tomcat 구현하기] 알파카(최휘용) 미션 제출합니다. #568
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robinjoon
approved these changes
Sep 6, 2024
There was a problem hiding this comment.
안녕하세요 알파카! 세번째 만남인게 신기하네요 ㅋㅋㅋ 1단계만 해주셔서 크게 리뷰를 드릴만한 부분이 없었어요. 그래서 메서드 이름같은 자잘하다면 자잘한 부분에만 신경을 쓰게 되었네요.
HTTP에 대한 이해나 스레드 대한 이해 같은 것 역시 이번 단계에서 짚고 넘어가기에는 요구사항이 워낙 제한적인 단계라 적절하지 않다고 생각해서 남기지 않았어요. 다음 단계 구현 뒤에 리뷰 요청 주실 때는 알파카의 학습을 위해 HTTP나 스레드에 대해 질문을 남겨보려 합니다. 제가 더 잘 안다고 생각해서 하는 것은 아니고, 같이 이야기해보면서 학습해보고자하는 의도에요.
Comment on lines
+18
to
+23
| InMemoryUserRepository.findByAccount(requestAccount) | ||
| .ifPresent(user -> { | ||
| if (user.checkPassword(requestPassword)) { | ||
| LOGGER.info(user.toString()); | ||
| } | ||
| }); |
Comment on lines
+46
to
+47
| String resourceName = requestMapper.requestMapping(request); | ||
| String responseBody = parseStartLine(resourceName); |
There was a problem hiding this comment.
변수랑 메서드 이름이랑 잘 맞지 않는 것 같아요. requestMapper 가 requestMapping 을 한다는게 어떤 의미인지 알기 어렵네요.
parseStartLine()의 결과가 responseBody인 부분도 마찬가지로 의아하네요.
Comment on lines
+22
to
+24
| public String getQueryStringData(String input) { | ||
| return queryString.get(input); | ||
| } |
There was a problem hiding this comment.
메서드 이름에 Data와 String이 둘 다 들어가는 부분이 어색하게 느껴지네요. 둘 중 하나만 들어가는건 어떤가요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 로빈! 벌써 세번째 만남이네요.. ㅎㅎ
시간 문제로 인해 1단계 및 리팩토링만 진행하고 제출하게 되었습니다.
조금이라도 개선이 필요해보이면 부담 없이 리뷰로 남겨주시길 바랍니다.
열심히 리뷰 반영해서 미션 잘 완성해보아요