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 구현하기 - 1단계] 호이(이건호) 미션 제출합니다 #418

Merged
merged 5 commits into from
Sep 15, 2023

Conversation

This2sho
Copy link

안녕하세요 말랑✨
이번 미션 잘 부탁드립니다~🧎🏻‍♂️

Copy link

@shin-mallang shin-mallang left a comment

Choose a reason for hiding this comment

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

안녕하세요 호이~
리뷰 너무 늦게 확인해서 죄송합니다 ㅠㅠ
코드 읽기 쉽게 작성해 주셔서 커멘트 달 부분이 거의 없었습니다.
간단하게 커멘트 달았으니 다음 단계에서 적용해주시면 감사하겠습니다 :)

Comment on lines +32 to +34
for (Object subPackage : basePackage) {
intTargetPackage(subPackage);
}

Choose a reason for hiding this comment

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

Object 배열을 생성자의 인자로 넣어주는 방법도 있었을 텐데 for문을 사용하신 이유가 따로 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

생성자 내에서 로직 처리를 말씀하시는건가요?!
이 부분은 Test에서 initialize() 메소드를 호출하는 부분이 있어서 그대로 구현했습니다.

}

private void initTargetMethod(final Class<?> clazz, final Method method) {
if (Objects.nonNull(method.getDeclaredAnnotation(RequestMapping.class))) {

Choose a reason for hiding this comment

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

Objects.nonNull() 메서드를 사용하신 이유가 있나요?
해당 메서드의 설명을 읽어보셨나요?

Copy link
Author

@This2sho This2sho Sep 15, 2023

Choose a reason for hiding this comment

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

== , !=를 직접 사용하는 것보다 Objects.nonNull() 메서드로 사용하는게 객체지향적인거라고 생각했습니드아
메서드 참조용으로 만들어진걸로 알고있습니다.

혹시 의도에 맞게 사용하는게 좋을까요?

Choose a reason for hiding this comment

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

굳이 의도에 맞게 사용할 필요는 없다고 생각합니다!
다만 답변에서 궁금한 부분이 생겨서 추가 질문 드릴게요! ㅋㅋㅋ
==, != 은 객체지향 적이지 않은데, Objects.nonNull()이 객체지향적이라 생각하시는 이유가 무엇일까요?
무엇을 기준으로 객체지향적인지를 판단하시는건지 궁금해요!

Copy link
Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋ 객체지향적인게 먼지 물어볼거 같아서 객체지향적이라말을 쓸까 말까했는데... 물어보시네요 (역시 말랑신)
이게 객체 지향적인게 맞는진 모르겠지만, ==, != 보다는 Objects라는 객체에게 nonNull 이냐고 메시지를 던지는게 조금 더 객체 지향적이라고 생각했습니다... 객체에게 메시지를 던져라..

Choose a reason for hiding this comment

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

오호 답변 감사합니더.
확실히 객체 지향적일 수는 있겠네요.
그런데 결국 어떠한 이점을 얻기 위해서 객체지향을 추구하는 것일 텐데, != null 보다 Objects.nonNull을 사용하여 얻을 수 있는 이점이 무엇이 있을까요?
(그렇게 중요하지는 않은 것 같은 내용인데, 너무 물고 늘어지는 것 같아 죄송하네요..😓)

Copy link
Author

@This2sho This2sho Sep 18, 2023

Choose a reason for hiding this comment

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

제가 생각하는 객체지향의 이점은 코드를 컴퓨터가 아닌 사람이 이해하기 쉽게 만들어서
유지 보수성(생산성) 향상이 제일 큰 이점인 거 같습니다.

현재 코드에서 Objects.nonNull(A)A != null 은 별 차이가 없는거 같네요... ㅠㅠ

private HandlerKey getHandlerKey(final Method method) {
final RequestMapping requestMapping = method.getDeclaredAnnotation(RequestMapping.class);
final String uri = requestMapping.value();
final RequestMethod requestMethod = requestMapping.method()[0];

Choose a reason for hiding this comment

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

이렇게 구현하면 Method를 여러개 설정하더라도 처음 메서드밖에 적용이 안되지 않나요?

Copy link
Author

Choose a reason for hiding this comment

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

아..! RequestMapping에 할당되는 메서드가 하나일거라고만 생각했는데
이전 미션에서 구현한 걸 생각해보니까 내부적으로 if문으로 처리해줬을 수 도 있겠네요..! ㄷㄷ

return new HandlerKey(uri, requestMethod);
}

private Object getInstance(final Class<?> clazz) {

Choose a reason for hiding this comment

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

getInstance보단 createHandler 와 같이 명확한 이름을 주는 거에 대해서는 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

좋습니다!

private Object getInstance(final Class<?> clazz) {
try {
final Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);

Choose a reason for hiding this comment

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

private 생성자를 대비하기 위해 넣으신건가요?

Copy link
Author

Choose a reason for hiding this comment

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

넵! JPA에서는 생성자가 최소 protected여야 한다.라는 스펙이 있지만, Spring에서는 prirvate이여도 빈등록이 가능해서 넣었습니다..

Choose a reason for hiding this comment

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

오호 좋네요~
미션에서 벗어나지만 추가적인 질문 드려도 될까요..? ㅋ꙼̈ㅋ̆̎ㅋ̊̈ㅋ̌̈ㅋ̄̈ㅋ̐̈ㅋ̑̈ㅋ̆̈

넵! JPA에서는 생성자가 최소 protected여야 한다.라는 스펙이 있지만

왜 생성자가 최소 protected여야 하는지 아시나요?

Copy link
Author

Choose a reason for hiding this comment

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

JSR 338에 The entity class must have a no-arg constructor... The no-arg constructor must be public or protected.라는 문장이... ㅋㅋ

JPA에서 DB에서 객체를 가져올 때 프록시 객체를 사용하는데, 프록시 객체는 Entity 객체를 사용하기 때문에 protected여야하는 걸로 알고 있습니다...!!

Choose a reason for hiding this comment

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

오호 문서까지 찾아주시다니.. 👍🏻👍🏻
제가 잘 몰라서 그런데 Entity 객체가 무엇일까요?
Entity 라는 클래스가 존재하나요?
추가적으로 프록시를 사용하려면 반드시 생성자가 protected여야 하나요?
private 생성자를 가지면 프록시로 만들 수 없을까요?

Copy link
Author

Choose a reason for hiding this comment

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

제가 잘 몰라서 그런데 Entity 객체가 무엇일까요?
??🥲 말랑이 모르는걸 제가 어떻게 알죠??

Entity는 저장할 수 있는 가벼운 도메인 객체 입니다...

앗.. 사용이 아니라 상속인데.. 잘못 작성했네여...
private 생성자를 사용하면 프록시 객체에서 super()로 Entity 클래스의 기본 생성자 호출이 불가능 할 거 같습니다!

@shin-mallang shin-mallang merged commit 98d7efc into woowacourse:this2sho Sep 15, 2023
1 check failed
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.

3 participants