🐱 서론
토이프로젝트를 하며 제출 기한에 맞추려다보니 미처 숙지하지 못한 구현부분에 대해서 얼레벌레 넘어가게 되었다.
또 다른분들의 코드를 참고하기도 하며(일부분 사실 카피;) 제출했는데
내게 의미없는 프로젝트로 남을 것 같아 미루고 미루다가 늦게나마 다시 살펴보았다.
🔎 수정 내용
1. 상속 대신 인터페이스로 구현
package me.day15.smartstore.customers;
public class ClassifiedCustomers extends Customers {package me.day15.smartstore.menu;
public class CustomerMenu extends Menu {처음 클래스 설계 시 위와 같은 상속을 이용하였는데,
이렇게 됐을 때 Menu 가 변경됨에 따라 CustomerMenu까지 변경되는 상황이 불가피하다.
그런데 인터페이스를 통해 공통기능을 추출하고 추상적으로 구현한다면 변경에 강하며 유지보수가 쉬워질 수 있다는 점을 알게되었고 클래스 관계를 다시 설계하였다.
public interface Menu {
Scanner scanner = new Scanner(System.in);
String input = scanner.nextLine();
int selectMenu = Integer.parseInt(input);
default String viewMenu(int selectMenu) {
return scanner.nextLine();
};public class CustomerMenu implements Menu {
static CustomerMenu customerMenu;
private final Customers customers;
private CustomerMenu(Customers customers) {
this.customers = customers;
}
public static CustomerMenu getInstance() {
if(customerMenu == null) {
customerMenu = new CustomerMenu(Customers.getInstance());
}
return customerMenu;
}대표적으로 Menu와 CustomerMenu의 수정코드를 보면 Menu를 상속이 아닌 인터페이스로 구현해주었고 선택적으로 오버라이딩하여 사용할 수 있도록 default 메서드로 정의해주었다.
또 CustomerMenu에서는 Menu를 implements 할 수 있고 customers 는 외부에서 변경이 불가하도록 private final 을 선언해주었다.
2. 클래스 별 적절한 메서드 배치
private static final int ID_LENGTH_MIN = 5;원래 Customer 에서 위와 같은 아이디값 검증 로직 메서드를 넣어줬었는데 이 메서드를 CustomerMenu 클래스에 배치하면 더 적절할 것 같아서 옮기려고했다.
private static final int ID_LENGTH_MIN = 5;
public Customer(String name, String userID, int spentTime, int totalPay) {
AUTO_GENERATOR++;
serialNO = String.format("%04d", AUTO_GENERATOR);
this.name = name;
this.userID = userID;
this.spentTime = spentTime;
this.totalPay = totalPay;
validationIdLength(userID);
this.userID = userID;
}
private void validationIdLength(String userID) {
if (userID.length() < ID_LENGTH_MIN) {
throw new IllegalArgumentException();
}
}그런데 검증은 객체를 생성하는 시점에 해주어야 적절한 예외를 던지고 예외처리를 통해 제어할 수 있다는 부분 때문에 어느 클래스에 넣어주는 것이 좋은지 조금 더 고민해봐야 할 것 같다. (name 도 마찬가지)
public void setGroupType() {
for(int i = GroupType.VVIP.ordinal(); i >= GroupType.GENERAL.ordinal(); i--)
{
if(false == Groups.getInstance().isGroupExist(GroupType.values()[i]))
continue;
int time = Groups.getInstance().getParameterByGroupType(GroupType.values()[i]).getMinimumSpentTime();
int money = Groups.getInstance().getParameterByGroupType(GroupType.values()[i]).getMinimumTotalPay();
if(this.spentTime >= time && this.totalPay >= money)
{
this.groupType = GroupType.values()[i];
return;
}
}
} public void setGroupType() {
this.groupType = groupType;
}또 기존 Customer 에서의 setGroupType() 은 totalPay 와 spentTime 을 비교하였는데 이를 단순히 값 변경하는 역할로만 바꿔주고
값 비교의 책임은 Groups 에서 가지는것이 좋을 것 같아 옮겨주었다.
이 과정에서 실제 비교 로직은 Parameter 내부로 숨겨두면 좋다는데 못숨겼다.
Groups 는 도메인 객체이기 때문에 출력 로직은 Menu 하위에 있는 객체에게 위임한다. (GroupMenu..?)
public int inputParameter(int chooseNum)
{
String message = "";
switch (chooseNum){
case 1 :
message = "Input Minimum Spent Time: "; break;
case 2 :
message = "Input Minimum Total Pay: "; break;
default : return -1;
}
int inputNumber = 0;
while(true)
{
System.out.println(message);
String input = UtilScanner.getInstance().getScanner().nextLine();
try {
inputNumber = Integer.parseInt(input);
if(inputNumber < 0)
System.out.println(Message.ERR_MSG_INVALID_INPUT_FORMAT);
else
break;
}catch (NumberFormatException e) {
System.out.println(Message.ERR_MSG_INVALID_INPUT_TYPE);
}
}
return inputNumber;
}
public boolean inputParameter(GroupType groupType)
{
final int MAX_OPTION_VALUE = 3;
while (true) {
System.out.printf(
"==============================\n" +
"1. Minimum Spent Time\n" +
"2. Minimum Total Pay\n" +
"3. Back\n" +
"==============================\n");
String input = UtilScanner.getInstance().getScanner().nextLine();
Exception e = UtilScanner.getInstance().checkInputValueException(input, 1, MAX_OPTION_VALUE);
if (null != e) {
System.out.println(e.getMessage());
continue;
}
int chooseNum = Integer.parseInt(input);
if(MAX_OPTION_VALUE == chooseNum)
break;
if(null == groups[groupType.ordinal()])
break;
int parameterValue = 0;
while (true) {
parameterValue = inputParameter(chooseNum);
if(-1 == parameterValue)
break;
if(!isGroupParameterValid(groupType, chooseNum, parameterValue))
{
System.out.println(Message.ERR_MSG_INVALID_INPUT_RANGE);
continue;
}
break;
}
switch (chooseNum)
{
case 1:
{
groups[groupType.ordinal()].setSpendTime(parameterValue);
}break;
case 2:
{
groups[groupType.ordinal()].setMinimumSpendMoney(parameterValue);
}break;
default:
break;
}
}
return true;
}3. ETC
-
Customer에서getInstance()함수가 반복호출되기 때문에 최상위 또는 메서드 최상위에서 변수로 꺼내놓기 (객체간의 함수 호출은 최대한 줄이는게 좋기 때문이다.)
-
값 비교 구문 수정
Arrays.sort(classifiedCustomers, (c1, c2)-> c1.getName().compareTo(c2.getName()));
Arrays.sort(classifiedCustomers, (c1, c2)-> c1.getTotalPay() - c2.getTotalPay());
Arrays.sort(classifiedCustomers, (c1, c2)-> c1.getSpentTime() - c2.getSpentTime());Arrays.sort(classifiedCustomers, Comparator.comparing(Customer::getName));
Arrays.sort(classifiedCustomers, Comparator.comparingInt(Customer::getTotalPay));
Arrays.sort(classifiedCustomers, Comparator.comparingInt(Customer::getSpentTime));람다함수가 더 간결하다고 생각했는데 아닌 것 같다.
아무튼 Comparator를 이용하여 객체를 비교/정렬 해준다.
❓ 여기서 Comparator 를 사용하는 이유
: Comparable , CompareTo 와 다르게 자기 자신이 아닌 매개변수로 들어오는 객체를 비교해주기 때문이다.
-
inputParameter처럼 메서드가 일정 길이 이상으로 길어진다면 자신만의 분리 기준을 세워 리팩토링 해준다.
-
Main의run()함수에서switch문 대신 가독성이 좋은if-return문으로 수정해준다.
🔄 개선이 필요한 나의 문제점
- 인터페이스와 추상화 개념 부족
- 비즈니스 로직과 출력 로직의 명확한 구분
- 커밋의 습관화와 커밋메시지 구체화
- 깔끔하지 않은 코드
- 불필요한 주석, 사용되지 않는 메서드 삭제 필수
- 인텔리제이 자동정렬을 활용하자
❓ 코딩 컨벤션이란
✅ 추가로 수정 할 부분
-
Scanner대신BufferReader사용하기 - 세부적인 예외처리 해주기
(if가 아닌try-catch로 예외처리를 넣어주기. 최상위Exception하나로 구현하기보다 구체적인, 커스텀의 특정 예외를 사용하기 )
