[톰캣 구현하기 - 3단계] 몰리(김지민) 미션 제출합니다. #653
Conversation
| final User savedUser = getvaliduser(account, requestPasswrd); | ||
| validateUserPassword(savedUser, requestPasswrd); |
There was a problem hiding this comment.
레포지토리에 findByAccountAndPassword 함수를 만들면 더 간단해질 것 같아요!
There was a problem hiding this comment.
오 그러네요 👍
그런데, findByAccountAndPassword를 사용해서 기존 로직을 대체하게 되면 아예 서비스에 없는 회원일 때와 존재하지만 비밀번호가 틀렸을 경우의 구분을 할 수 없을 것 같아요...!
요고에 대해서는 산초가 어떻게 생각하시는지 궁금해요!
tomcat/src/main/java/org/apache/coyote/controller/AbstractController.java
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/controller/StaticResourceController.java
Show resolved
Hide resolved
| public Builder addHeader(HttpHeaderName name, String value) { | ||
| headers.add(new HttpHeader(name, value)); | ||
| return this; | ||
| } | ||
|
|
||
| public Builder addLocation(String value) { | ||
| headers.add(new HttpHeader(HttpHeaderName.LOCATION, value)); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
[반영 안해도 되는 제안]
그냥 header() location() 으로 명명하면 어떨까요?
물론 지금 add 로 시작하는 함수와 그렇지 않은 함수들이 내부적으로 다르게 동작하긴 하지만, 함수를 사용하는 사람 입장에서는 그런 것들을 고려하지 않을 것 같아요.
저도 이 부분의 코드만 읽을 때는 그냥 넘어갔었는데, 뒷부분에서 addLocation() 함수를 마주하고 조금 혼란이 있었어요!
하지만 몰리가 이렇게 명명하신 이유가 타당하다고 스스로 느끼신다면 반영 안하셔도 됩니다!
There was a problem hiding this comment.
일단 addHeader 라는 메서드는 한 HttpResponse 당 헤더�는 여러개 존재할 수 있고,
인자를 새로 대입을 하는 것이 아닌 기존 헤더 목록에 인자로 받은 헤더를 추가하는 것을 의도했기 때문에 이름을 다음처럼 작성한 것 같아요.
개인적으로 headers(), header() 같이 작성되면 뭔가 기존 값이 덮히고 인자값만 남는 느낌이라는 생각이 들어서 어색했던 것 같습니다...! (저만 그렇게 느끼는 걸까요...? 🥲 아니라면 말해주세요!)
그런데 addLocation은 산초 말처럼 충분히 헷갈릴 수 있을 것 같아요!
수정했습니다!
안녕하세요 산초!
PR이 많이 늦었네요 ㅠㅠ
시간이 충분하지는 않지만 많은 이야기 나누고 싶어요!
이해가 되지 않으시는 부분 있다면 편하게 말씀 주시면 감사하겠습니다!
이번 PR도 잘 부탁드립니다! 🔥