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

[1차 VER1.0] Java ToyProject upload by TaeIlKIM #14

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

Conversation

james-taeil
Copy link

코드리뷰 부탁드립니다.

  • 일단 잘 짠 코드인지 확신이 안섭니다.
  • 기능은test mode와 동일하게 작동하는 것 확인했습니다.
  • summary 쪽 comparator 을 사용하는 과정에서 개념이 재대로 잡혀 있지 않은 것 확인했습니다.

긴 코드 인데 읽어주셔서 너무 감사합니다! 🙏


TODO - Refactoring

  • Summary sort 반복적인 코드 일관성 있게 리펙토링 필요
  • TEST코드 구현 필요
  • 반복되는 코드 객체지향적으로 풀이 필요


private Customers() {};

public void refresh() {

Choose a reason for hiding this comment

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

refresh 메서드를 오버로딩해서 한 고객에 대해서만 업데이트하는 메서드가 있으면 좋을 것 같습니다!
고객 한 명을 추가했는데 모든 고객을 refresh한다면 비효율적이니까요 😀

Copy link
Author

Choose a reason for hiding this comment

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

그런 생각 안해봤는데, 참고해보겠습니다~

return customerArrs;
}

public Comparator<Customer> sortName(int num) {

Choose a reason for hiding this comment

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

람다식으로 할 생각은 전혀 못했었는데 덕분에 하나 배워갑니다!👍

try {
System.out.println("Input Customer's Spent Time: ");
Integer time = Integer.parseInt(nextLine(Message.END_MSG));
if (time < 0) throw new InputRangeException();
Copy link

@seonghye0n seonghye0n May 11, 2023

Choose a reason for hiding this comment

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

setter에 해당 조건이 들어가면 더 좋을 것 같다는 생각이 듭니다!
다른데서 해당 조건 없이 setter를 쓴다면 결국 잘못된 값이 입력이 되지 않을까요?!

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