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

[2주차] 객체지향 코드 연습(BaekJaehyuk) #9

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

BaekJaehyuk
Copy link

추가 Q & A

🤔 잘 모르겠거나, 더 궁금한 내용이 있었나요?

집중적으로 리뷰 받고싶은 것이 있나요?

MVC 패턴을 지키고자 하였지만 구현하고 보니 MVC 패턴의 장점인 관심사의 분리를 지키지 못하였고, SOLID의 원칙을 지키고 못하였다. 앞으로의 리팩토리에 있어 이러한 부분을 고쳐나가고 싶습니다. 또한, 코드가 전체적으로 깔끔하지 못하여 아쉬움이 크게 남아있어 이에 관련해서 피드백 받고 싶습니다.

Copy link
Contributor

@KoSeonJe KoSeonJe left a comment

Choose a reason for hiding this comment

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

변수명, 메소드명 클래스 명을 많이 고민해서 명확하게 어떤 일을 하고 있는지 나타낼 수 있어야 합니다.

의존성이 외부로부터 주입되지 않고 있습니다. 이것은 클래스와 클래스간의 결합도를 높이기 때문에 외부에서 주입하는 것이 좋습니다.

도메인 별 패키지 분리를 고민해봐야 할 것 같습니다.

선언한 클래스명과 달리 다른 책임을 수행하는 경우가 많은 것 같습니다. 책임을 적절하게 분리해주면 좋을 것 같습니다

src/main/java/banking/account/view/BankView.java Outdated Show resolved Hide resolved
src/main/java/banking/account/view/BankView.java Outdated Show resolved Hide resolved
src/main/java/banking/account/view/BankView.java Outdated Show resolved Hide resolved
src/main/java/banking/account/view/BankView.java Outdated Show resolved Hide resolved
src/main/java/banking/account/view/BankView.java Outdated Show resolved Hide resolved
src/main/java/banking/account/service/BankService.java Outdated Show resolved Hide resolved
src/main/java/banking/account/service/BankService.java Outdated Show resolved Hide resolved
src/main/java/banking/account/service/Service.java Outdated Show resolved Hide resolved
src/main/java/banking/account/repository/CentralBank.java Outdated Show resolved Hide resolved
src/main/java/banking/account/repository/CentralBank.java Outdated Show resolved Hide resolved
Copy link

@Gopistol Gopistol left a comment

Choose a reason for hiding this comment

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

View가 가진 역할에 대해 고민해보시면 좋겠습니다! 비즈니스 로직에 관련된 부분과 View가 담당하는 부분을 구분하는 것이 필요해 보입니다.

src/main/java/banking/account/dto/AccountDTO.java Outdated Show resolved Hide resolved
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