Skip to content

Lee ji hyung#1

Open
groundinsider wants to merge 13 commits intoLikeLion-Study:masterfrom
groundinsider:LeeJiHyung
Open

Lee ji hyung#1
groundinsider wants to merge 13 commits intoLikeLion-Study:masterfrom
groundinsider:LeeJiHyung

Conversation

@groundinsider
Copy link
Copy Markdown

@groundinsider groundinsider commented May 4, 2025

시간이 없어서 추가 기능은 구현 못했습니다...
MVC 구조를 처음 해봐서 흉내만 내봤는데 개선점이 무엇인지 궁금합니다

Copy link
Copy Markdown
Member

@UiHyeon-Kim UiHyeon-Kim left a comment

Choose a reason for hiding this comment

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

3주차 고생하셨습니다~

전반적인 코드는 잘 작성하신 것 같아요.
만약 MVC 패턴을 적용하고 싶으시다면, 따로 View 패키지를 만들어 입출력만 담당하게 하면 좋을 것 같아요. repository는 CRUD (즉, Model)를 담당하고, service는 비즈니스 로직을 담당을 하고 있습니다. 여기서 출력 부분만 따로 빼주신다고 생각하면 되는데, 이 또한 내부에 객체를 생성하는 것보다 생성자 주입하는 것이 OOP 를 지키기 위한 길이라고 생각이 듭니다.


public class CustomerService {
ArrayList<CustomerLegacy> customers = new ArrayList<>();
UserService userService = new UserServiceImpl();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

UserService를 인터페이스로 추상화 한 것은 매우 좋다고 생각합니다.
그런데 인터페이스의 구현체를 CustomerService 내부에 객체로 생성하면 인터페이스로 추상화 한 이점이 살지 않습니다.

또한 비즈니스 로직같은 상위 모듈은 데이터, 모델 같은 하위 모듈을 직접 참조하면 안됩니다.
객체지향 원칙에는 DIP 라는 것이 있는데 한 번 찾아보시면 도움이 되실 것 같아요.

import java.util.ArrayList;

public class UserServiceImpl implements UserService {
UserRepository userRepository = new UserRepositoryImpl();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

여기 또한 마찬가지겠죠?

ArrayList<User> users = userRepository.getUsers();
System.out.println("----- 고객 조회 결과 -----");
System.out.println("총 고객 수: " + users.size());
int i = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

대부분의 변수명은 괜찮았는데 이부분은 조금 아쉽네요.
제가 문제를 알고 있어서 금방 이해할 수 있었지만, 몰랐더라면 조금 아쉬운 코드가 될 것 같아요.

그리고 번호가 1번부터 시작해야 하는 것으로 아는데 i++ 로 진행하면 0부터 나올 것 같은데 한 번 확인해 보시면 좋을 것 같아요.

}
else{
status = "비활성";
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

자바를 사용한다면 이런 간단한 조건은 3항 연산을 사용하면 더 깔끔하게 보일 것 같아요!

그리고 toString을 활용한 것이 굉장히 좋아 보입니다!

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