[1차] Java ToyProject upload by TaeHyoungSong #23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
각 기능으로 이동하는 메뉴 구현
CusNo를 선언하고 대입하여 사용하였지만 삭제 시에 CusNo를 별도로 선언 하지않아 문제 발생
Ussu1112
changed the title
hyunsb
reviewed
May 12, 2023
src/me/smartstore/SmartStoreApp.java
Outdated
Comment on lines
13
to
15
| private final Groups allGroups = Groups.getInstance(); | ||
| private final Customers allCustomers = Customers.getInstance(); | ||
| private final MainMenu mainMenu = MainMenu.getInstance(); |
There was a problem hiding this comment.
의존성 주입부분은 저도 어떻게 해야될 지 잘 모르겠는데
같이 고민해볼까요?
Author
There was a problem hiding this comment.
싱글톤 객체를 주입받는 방식에 대해 찾아봤습니다...🤔
생성자 주입방식과 setter 주입방식를 자주 쓰지만
setter 주입방식은 코드의 유연성에 초점이 맞춰져 있는듯 하고,
생성자 주입방식은 객체의 불변성이나 코드의 가독성에 더 좋은듯합니다.
토이프로젝트에서는 생성자 주입방식이 더 낫지 않을까요?
private Groups allGroups;
private Customers allCustomers;
private MainMenu mainMenu;
private SmartStoreApp() {
this.allCustomers = Customers.getInstance();
this.allGroups = Groups.getInstance();
this.mainMenu = MainMenu.getInstance();
}
hyunsb
reviewed
May 12, 2023
Comment on lines
48
to
53
| String cusName = null; | ||
| String cusId = null; | ||
| int cusTotalTime = 0; | ||
| int cusTotalPay = 0; | ||
|
|
||
| Customer customer = new Customer(cusName, cusId, cusTotalTime, cusTotalPay); |
There was a problem hiding this comment.
멘토님이 짚어주신 부분이었는데
Customer의 초기값을 설정하는 역할은 누가 가지는 게 맞을까요?
Author
There was a problem hiding this comment.
Customer의 초기값을 설정해주는 부분은 Customer의 생성자 쪽으로 넘겨서 처리해보았습니다.
변수들은 allCustomers에 set해주기 위해서 사용하는데 다시보니 굉장히 지저분해 보이네요.
set하는걸 별도로 빼는게 좋을까요?
ecsimsw
approved these changes
May 24, 2023
|
|
||
| for (int i = saveIdx; i < allCustomers.size(); i++) { | ||
| Customer customer = allCustomers.get(i); | ||
| System.out.println(customer); |
| System.out.println("Which order (ASCENDING (A), DESCENDING (D))?"); | ||
| String order = nextLine(Message.END_MSG); | ||
| if (order.equals("A") || order.equals("D")) return order; | ||
| else if (order.equals("end")) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
토이프로젝트 제출합니다!
항상 코드리뷰 감사합니다.
이번 프로젝트를 진행하면서 아직도 객체 간의 관계에 대해 코딩하는 방법에 대해 조금 미숙한 듯 합니다.
코드를 더하면 더할수록 점점 주먹구구식으로 코딩을 하는 것 같아 어지럽습니다 🤣
보면 볼수록 코드를 어떻게 수정하면 더 괜찮은가에 대해 고민하게 되는 토이프로젝트 였습니다.
코드 리뷰와 피어 리뷰를 하면서 리팩토링을 계속 진행해보겠습니다!