Open
Conversation
sah3122
requested changes
Aug 15, 2024
sah3122
left a comment
sah3122
left a comment
There was a problem hiding this comment.
Controller 인자 매핑 구현 잘해주셨네요 👍
몇가지 고민거리를 남겨두었는데 확인 부탁드립니다 🙏
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.lang.reflect.Parameter; | ||
|
|
||
| public interface ArgumentResolver { |
| import java.util.List; | ||
|
|
||
|
|
||
| public class MethodArgumentResolvers { |
| public class PathVariableResolver implements ArgumentResolver { | ||
| private final String urlPattern; | ||
|
|
||
| public PathVariableResolver(String urlPattern) { |
There was a problem hiding this comment.
urlPattern 을 상태 값으로 가지고 있는다면 해당 PathVariableResolver 는 특정 UrlPattern만을 지원할수 있는 Resolver로 역찰이 제한되게 될것 같습니다.
특정 패턴에 제한하지 않고 stateless한 클래스로 정의하여 한번 생성으로 재활용할수 있도록 변경해보는건 어떨까요 ?
Author
There was a problem hiding this comment.
넵 ArgumentResolver에서 Method 파라미터를 추가하여 구현하는것으로 변경했습니다!
Comment on lines
+27
to
+30
| new HttpServletRequestResolver(), | ||
| new HttpServletResponseResolver(), | ||
| new PathVariableResolver(urlPattern), | ||
| new RequestParamResolver() |
There was a problem hiding this comment.
create 함수가 호출될때 마다 매번 생성하도록 정의해주셨는데, 재활용 할수 있는 구조로 변경해보는건 어떨까요 ?
| public class TypeCheckUtil { | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| public static <T> T convertStringToTargetType(String value, Class<?> targetType) { |
There was a problem hiding this comment.
지원하는 원시 타입 정의 👍
개인적으로 조건문을 나열하기 보단 Enum 형태로 관리할수 있을듯 한데 도전해보세요 👍
| public Object resolve(HttpServletRequest request, HttpServletResponse response, Parameter parameter) { | ||
| return request; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
사소하지만 파일의 마지막은 new line으로 끝나는것을 권장드립니다. 😄
Author
|
피드백 내용 반영했습니다! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
안녕하세요 동철님!
마지막 4단계 구현했습니다.항상 피드백 감사합니다!