[후_Hwi] 웹서버 4단계 - 쿠키를 이용한 로그인 구현 #75
Open
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
- 디미터의 법칙에 맞게 User가 패스워드를 판별하게 만듦 - RequestHandler의 setRequestBody 메소드에서 if else 문을 삼항연산자로 변경
- RequestHandler에서 Request에 관한 자료들을 관리하는 클래스 분할
- RequestHandler에서 Reponse에 관한 자료들을 관리하는 클래스 분할
- Controller는 인터페이스로 분리함 - DefaultController 클래스 - LogInController 클래스 - LogOutController 클래스 - SignUpController 클래스
- 기능들을 실행하는 클래스로 수정함
wheejuni
requested changes
Apr 3, 2022
| public Response run(Request request) { | ||
| String userId = request.findBodyByFieldName("userId"); | ||
| String password = request.findBodyByFieldName("password"); | ||
| Optional<User> user = DataBase.findUserById(userId); |
There was a problem hiding this comment.
아래쪽 로직을 보니 .get() 을 호출하고 있던데 옵셔널 객체를 강제 언래핑하는 것은 피해 주시고요.
User 가 이 로직 안에서 Optional 상태로 존재할 이유가 있나요? 저라면 여기서 .orElseThrow() 와 같은 편의 메소드를 사용할 것 같아요.
| } | ||
|
|
||
| @Override | ||
| public Response run(Request request) throws IOException { |
|
|
||
| @Override | ||
| public Response run(Request request) { | ||
| byte[] body = "".getBytes(); |
Comment on lines
+44
to
+49
| private Runnable redirectSignUpSuccess(Request request, Map<String, String> responseHeader) { | ||
| return () -> { | ||
| createUser(request); | ||
| responseHeader.put("Location", "/index.html"); | ||
| }; | ||
| } |
Comment on lines
+46
to
+48
| httpMethod = requestInfo[METHOD]; | ||
| requestUrl = requestInfo[URL]; | ||
| httpVersion = requestInfo[HTTP_VERSION]; |
There was a problem hiding this comment.
굉장히 굉장히 사이드 이펙이 발생하기 쉬운 구조로 보이고요...
HttpRequestInfo 라는 클래스가 설계되어서, 메서드 간에는 객체의 형태로 넘나들어야 하지 않나 생각합니다.
그러니까 HttpRequestUtils.getRequestInfo() 의 리턴 타입으로 쓸만한 클래스 한 개,
적합한 파라메터를 넘겨받는 다른 private 메서드 n개가 설계돼야 할 것 같군요.
|
|
||
| private void setRequestHeader(BufferedReader input) throws IOException { | ||
| String line; | ||
| while (!"".equals(line = URLDecoder.decode(input.readLine(), StandardCharsets.UTF_8))) { |
There was a problem hiding this comment.
.isEmpty(), .isBlank() 와 같은 메서드들이 이미 String 에 있습니다.
| @@ -0,0 +1,55 @@ | |||
| package util; | |||
|
|
|||
| public class Pair { | |||
There was a problem hiding this comment.
데이터를 보관하는 꽤 중요한 클래스를 설계하셨는데 테스트가 한 줄도 보이지 않는군요.
Comment on lines
+4
to
+5
| String key; | ||
| String value; |
There was a problem hiding this comment.
접근제어자 누락입니다. 이런 클래스일수록 캡슐화와 은닉에 집중해야 합니다.
Comment on lines
+8
to
+9
| this.key = key.trim(); | ||
| this.value = value.trim(); |
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.
안녕하세요~ 후 앤 Hwi입니다.
3단계 요구사항 구현 완료하여 PR 보냅니다.
저희 코드를 보시고 리뷰 남겨주시는 분들께 감사드립니다.
자세한 내용은 README를 참고해주세요!
행복한 하루 되세요!😄