Skip to content
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단계] 로지(윤가영) 미션 제출합니다. #564

Merged
merged 12 commits into from
Sep 25, 2023

Conversation

kyY00n
Copy link
Member

@kyY00n kyY00n commented Sep 24, 2023

급하게 3단계를 구현했습니당..
사실 3단계는 그닥 고민을 하지 못했어요.
2단계에서 시도했던 mvc 모듈로 DispatcherServlet을 분리하는 과정에서의 의문이 아직 해결이 되지 않았기 때문인데요..
#466 (comment)
제 학습이 부족한 탓인지.. 단시간에 정복하기엔 어려운 내용인건지.. 아직 이해를 하지 못했어요.
호이와 이야기하면서 해결해보고싶은데 괜찮을까요?🥹

Copy link

@This2sho This2sho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

급하게 구현하셨다고 하셨지만... 잘하셨네요 로지 👍
#466 (comment) 이 부분은 저도 사실 왜 안되는지 이해가 안되서 계속 해봤는데.. 아직도 왜 그런지 모르겠네요..
같이 해결해보시죠..🥹

(너무 바로 approve만 한거 같아서.. 한번만 request change로.....🥲)


/**
* Base class for {@link WebApplicationInitializer} implementations that register a {@link DispatcherServlet} in the
* servlet context.
*/
public class DispatcherServletInitializer implements WebApplicationInitializer {
public class AppInitializer implements WebApplicationInitializer {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오홍 DispatcherServletInitializer 에서 AppInitializer 으로 이름을 바꾼 이유가 있을까유?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아, 별의미는 없었습니다. 이건 저의 아쉬움이 반영된 변경사항이었어요.
DispatcherServlet이 아니라 App으로 변경하면 app 모듈에 조금 더 직관적이지 않을까? 생각했어요.
애초에 저 이름이 있었어서 계속 mvc 모듈로 옮기려고 집착한 것 같다는 이유도 있습니다.. 하하.


public class DispatcherServlet extends HttpServlet {

private static final long serialVersionUID = 1L;
private static final Logger log = LoggerFactory.getLogger(DispatcherServlet.class);

private List<HandlerMapping> handlerMappings;
private final List<HandlerMapping> handlerMappings;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일...급..컬랙션..?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오오, 잊고있었던 일급컬렉션... 적용해보겠습니다

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다!

@This2sho This2sho merged commit 21adae4 into woowacourse:kyy00n Sep 25, 2023
1 check passed
Copy link

@This2sho This2sho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다..!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants