피드백을 주고받은 PR & 작성 코드
피드백 정리
1. 상황에 맞는 적절한 예외를 던지자
public class Deck {
...
public Card drawCard() {
if (cards.isEmpty()) {
throw new IllegalStateException(ERROR_EMPTY_DECK);
}
return cards.poll();
}
...
}
저는 지금까지 앞선 미션을 진행하면서는 발생하는 예외상황들을 모두 IllegalArgumentException
을 사용하였습니다.
그러면 좋지 않다는 것을 알면서도..상황별로 어떤 예외를 쓰는 것이 올바른지 모르기에 그랬던 것 같습니다😅
이번 피드백으로 상황별로 적절한 예외를 발생시켜주는 것이 좋을 것 같다는 피드백을 받아 상황별 필요한 예외에 대해 학습할 수 있었습니다. 이것만으로도
학습자료
- Effective Java Item72. 표준 예외를 사용하라 자세한 내용은 이펙티브자바 Item72부분을 참고하시면 될 것 같습니다.
1.1 메서드에서 아예 지원하지 않는 경우 예외상황
- 코멘트 링크
메서드에서 아예 지원하지 않는 경우에는 exception을
UnsupportedOperationException
를 사용하는 것이 적절하다.
2. 인스턴스를 생성할 일이 없는 상속용 클래스는 추상 클래스로 만들어라
해당 피드백은 아직까지 상속에 대한 지식이 부족하여 받은 피드백입니다.
추상 메서드가 없는 클래스이더라도 상속만을 위해 사용하는 클래스들은 abstract
를 붙여줌으로써
프로그램 내에서 해당 클래스의 인스턴스 생성을 막아줄 수 있습니다.
학습 정리 자료
3. View에서는 도메인의 메인 로직을 실행시키지 말자
public class OutputView {
...
public static void printFinalScore(DealerResult result, Users users, int dealerSum) {
printDealerResult(result.getCount());
for (User user : users.getUsers()) {
printUserResult(user, user.checkResult(dealerSum));
}
...
}
...
}
해당 피드백은 승패를 계산하는 checkResult()
와 같은 핵심 메서드가 도메인, 컨트롤러가 아닌 뷰에서 호출되어 사용되었기에 받은 피드백입니다.
저는 해당 피드백을 고치기 위해 DTO를 생성하여 도메인 또는 컨트롤러에서 실질적인 핵심 로직을 실행시킨 후, DTO를 통해 결과를 반환하고 뷰에서는 받은 결과를 출력하는 작업만 하도록 수정하였습니다.
4. Domain에는 View와 관련된 내용이 없도록 신경쓰자
public enum CardNumber {
ACE(11, "A"),
TWO(2, "2"),
THREE(3, "3"),
FOUR(4, "4"),
FIVE(5, "5"),
SIX(6, "6"),
SEVEN(7, "7"),
EIGHT(8, "8"),
NINE(9, "9"),
TEN(10, "10"),
JACK(10, "J"),
QUEEN(10, "Q"),
KING(10, "K");
private final int number;
private final String originalName;
...
아마 대부분의 사람들이 이번 미션을 진행하며 카드의 타입과 번호를 다음과 같이 enum타입으로 도메인 부분에 생성하여 사용했을 것입니다.
하지만 위의 피드백을 받고 현재 코드는 카드 출력의 형식을 변경한다면 view가 아닌 domain을 수정해야 한다!
는 사실을 꺠닫게 되며,
originalName
의 경우는 도메인이 아닌 뷰에 위치하는 것이 MVC입장에서는 맞다 생각했습니다.
그래서 저는 뷰에 아래의 OriginalCardNumber
, OriginalCardType
이라는 클래스를 생성하여 카드 타입,번호를 받고 그에 맞는 출력 타입을 반환하도록 코드를 수정했습니다.
public enum OriginalCardType {
HEART(CardType.HEART, "하트"),
DIAMOND(CardType.DIAMOND, "다이아몬드"),
SPADE(CardType.SPADE, "스페이드"),
CLOVER(CardType.CLOVER, "클로버");
private final CardType cardType;
private final String name;
OriginalCardType(CardType cardType, String name) {
this.cardType = cardType;
this.name = name;
}
public static String getOriginalName(CardType cardType) {
return Arrays.stream(values())
.filter(c -> c.getCardType() == cardType)
.map(OriginalCardType::getName)
.findAny()
.orElseThrow(NullPointerException::new);
}
private CardType getCardType() {
return cardType;
}
private String getName() {
return name;
}
}
5. 상수의 위치 및 접근 제어자
첫 번째 자동차 미션부터 이번에 한 블랙잭 미션까지 항상 상수에 대한 고민들을 많이 했습니다.
상수만을 위한 클래스를 생성하여 상수를 보관한다면 나중에 프로그램의 크기가 커졌을 때, 상수 파일의 크기가 수천 줄이 될 수 있어서 저는 상수를 각각의 사용되는 파일들에 private
으로 정의하고 사용했습니다.
하지만 이렇게 보관을 하면서도 몇몇 문제점이 있었습니다. 한개의 클래스에서만 사용하는 상수는 문제가 없지만 여러 클래스에서 공통적으로 사용되는 값들은 어떻게 관리를 하는 것이 좋을지 의문이더군요.
그래서 자동차 경주 미션의 리뷰어 미르와 이번 블랙잭 미션의 리뷰어 철시에게 상수 관리에 대해 물어본 결과 다음과 같았습니다.
-
실무에서는 상수를 공통으로 쓰는 팀도 있고 각 클래스마다 정의하는 팀도 있고 다양하다. -> 팀 또는 자신만의 룰을 만들어 통일만 하면 된다.
- 미르는 각각의 책임이 있는 클래스들에 위치시킨다고 했다.
- 공통으로 쓰이지만 자주 변경이 일어나지 않는 상수는 별도로 각 클래스에 정의할 것 같다고 했다.
- 철시는 프로그래밍을 하면서 가장 피해야하는 것 중 하나가 중복이라 생각하여, 의미가 같은 상수들이면 적절한 클래스에 정의한 후
public
으로 열어서 사용하는 것이 좋다 생각한다.
- 이프, 마루와도 코드 리뷰를 하는 시간을 가져봤는데 이프는 게임 내에 전체적으로 사용되는 상수들은
enum
으로 만들어 사용한다고 했습니다.
이 외에는 네이밍, 접근제어자 관련 간단한 피드백을 받은 것 같습니다.