-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MVC 구현 - 3단계] 베로(김은솔) 미션 제출합니다. #553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 베로!
3단계 미션도 잘 구현해주셨군요!
ApplicationContextAware
를 통해서 빈을 가져와서 사용한게 인상깊었네요 ㅎㅎ
의존성과 객체 분리에 대해 고민한 흔적들이 코드에 잘 담겨있었던것 같아요
ViewResolver와 관련해서는 저는 ViewResolver 로 분리하는 것도 좋을거같다는 생각이에요
또 해봐야 어떤게 문제가 있고 더 좋은지에 대해서 알 수 있다고 생각해 미션코드와는 조금 멀어지더라도 충분해 시도해볼만하다고 생각합니다! 리뷰하면서 많이 배우고 가네요 👍
이번 3단계 미션도 고생하셨습니다~!
@Override | ||
public String getName() { | ||
return viewName; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View 인터페이스에서 존재하는 해당 메서드는 구현체에서도 이제 사용하지 않는것 같은데 따로 그대로 두신 이유가 있나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삭제하는 것을 잊었네요😅 바로 삭제했습니다 👍
public ApplicationContext(final String externalPackage) { | ||
this.beans = new HashMap<>(); | ||
registerBeans(INTERNAL_BASE_PACKAGE); | ||
registerBeans(externalPackage); | ||
initialize(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public으로 registerBeans
메서드와 initalilize
메서드를 열어두었더라고요!
그래서인지 생성자의 파라미터를 가변인자로 받아도 좋을거같다는 생각이 들었네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 내부 패키지와 외부 패키지를 분리하려고 가변 인자로 받지 않고 각각 받았습니다!
그런데 스프링 코드를 보니 실제로는 가변인자로 가져오고 있네요 🧐
AnnotationConfigServletWebApplicationContext
를 보면, scan 이라는 함수가 있습니다.
@Override
public final void scan(String... basePackages) {
Assert.notEmpty(basePackages, "At least one base package must be specified");
this.basePackages = basePackages;
}
이런 것처럼 basePackages 들을 가변인자로 받는 걸 보면, 가변 인자로 받아도 상관 없을 것 같네요!!
public class JsonWriterObjectMapper { | ||
|
||
private static final ObjectMapper objectMapper = new ObjectMapper(); | ||
|
||
public static String writeJson(final Map<String, ?> objects) { | ||
try { | ||
return objectMapper.writeValueAsString(objects); | ||
} catch (JsonProcessingException e) { | ||
throw new RuntimeException("Json 객체 변환 과정에서 예외가 발생했습니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
따로 클래스 분리를 하신 이유가 있으실까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
변환 과정은 다른 객체에서 따로 하는 게 좋아보여서 분리했었는데, 지금 보니 굳이 분리되지 않아도 되는 동작 같네요. JsonView
자체가 뷰를 렌더링 해주는 역할을 하니까 ObjectMapper
도 클래스 내부에 있어도 될 것 같아요 :)
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class RedirectView implements View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redirectView 좋은거같아요 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
머지가 늦었네요! mvc 미션 고생하셨어요~
다음 미션도 화이팅입니다!
안녕하세요, 루쿠! MVC 3단계는 구현이 좀 늦었습니다 😅
ApplicationContext
는 한 번에 자동으로 만들어져야 하는 패키지들의 클래스들을 한 번에 읽어와서 bean 처럼 등록하는 역할을 합니다.ApplicationContext
를 사용해서 빈들을 가져와야 하는 클래스들은ApplicationContext
를 필드로 가지고 있는ApplicationContextAware
를 상속 받아 빈들을 가져올 수 있도록 했습니다. 최대한 리플렉션은ApplicationContext
에서만 일어나는 것을 신경써서 구현해 봤습니다 😄 루쿠가 구현하셨을 내용도 궁금하네요 ㅎㅎ초반에는 ViewResolver 를 구현해서 ViewResolver 가 각각 View 들의 특징을 구분해주도록 만들려고 했는데, 미션 코드에서 컨트롤러 단에서 직접 View 를 생성하는 것을 보고 다 삭제했습니다. 이 부분은 루쿠 의견이 궁금해요. 미션 코드와는 조금 달라지더라도 ViewResolver 로 분리하는 것이 좋았을까요?
패키지를 바꾸고 이것저것 수정하다보니 커밋 기록이 좀 많이 더러워졌습니다 죄송해요 😭 커밋 단위로 보기는 어려우실 것 같아 미리 사과드립니다 🙇♀️
시간 되실 때 리뷰해주세요! 감사합니다 :)