개요
우테코 2단계 미션 강의 내용 중 들은 것을 정리해 보았다
가독성 높은 코드
코드를 통해 의도를 전달해야 한다
int p = 0 ;위와 같은 코드가 있다고 해보자.
직관적으로 봤을 때, 다음 코드가 무엇을 의미하는지를 예상할 수 있는가?
// 위치
int p = 0 ;위처럼 주석을 통해 의도를 전달할 수 있겠지만, 코드 그 자체로 설명되도록 코드를 작성해야 유지보수 비용이 줄어든다
int position = 0 ;따라서 다음과 같이 네이밍으로 설명을 잘 해야할 필요성이 있다
class Position(){}위처럼 아예 원시값으로 포장해주면 더 비즈니스에 종속적일 수 있다.
일관된 코드 스타일을 가져가라
class Car {
private String name;
private int position;
public String getName()
{
return name;
}
public int position() {
return position;
}
public void Forward()
{
position += 1;
}
public void back() {
position--;
}
}
- 컨벤션이 맞지 않는다
- getter가 맨 위에 있다
- 연산자가 제각각이다
- 네이밍에서 의도가 드러나기 않는다
-> 종합적으로 개발자의 의도가 드러나지 않아, 코드를 읽는 사람 입장에서 뇌에 혼란을 준다
-> 따라서 일관된 코드 스타일을 가져가야 할 필요성이 있다
하나의 역할만을 담당하라
class Lotto {
// 우승 로또 번호와 비교하여 상금을 얻는다.
Money calculatePrize(final WinningLotto winningLotto) { }
}- 번호 비교 책임 + 상금 반환 책임 이 한 메소드에 들어가 있다
- 책임이 단순 두 가지 일지라도 이곳에는 상당히 많은 계산 비용이 소모된다
-> 추상화 계층이 커지고, 테스트코드 작성이 어려워질 것이다
매개변수를 명확하게 하라
final var neo = new Crew(
"neo",
10_000,
List.of("쌈밥", "김치찌개", "탕수육", "비빔밥"),
List.of("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스")
);final var neo = new Crew(
name: "neo",
balance: 10_000,
likeMenus: List.of("쌈밥", "김치찌개", "탕수육", "비빔밥"),
dislikeMenus: List.of("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스")
);- 매개변수가 명확하지 않으면 이해하기 어렵다. 자바는 네임드 파라미터를 지원하지 않기 때문이다. 인텔리제이에서 힌트를 주긴 하지만, 어디서나 인텔리제이를 쓴다는 보장은 없다. 또 깃허브 플랫폼 속에서 코드리뷰를 한다면 저런 힌트를 볼 수 없다.
final var neo = new Crew(
/*name*/ "neo",
/*balance*/ 10_000,
/*likeMenus*/ List.of("쌈밥", "김치찌개", "탕수육", "비빔밥"),
/*dislikeMenus*/ List.of("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스")
);- 이렇게 주석 처리를 해주면 의미를 드러낼 수는 있지만, 역시 코드 자체로는 드러나지 않는다는 한계가 있다
final var taste = Taste.builder()
.like("쌈밥", "김치찌개", "탕수육", "비빔밥")
.dislike("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스")
.build();
final var neo = new Crew(
"neo",
new Balance(10_000),
taste
);- 위처럼 빌더 패턴을 쓸 수도 있다. 그러나 데이터가 추가되면 될수록 여전히 이해하기 어려울 것이다. 나중에 파라미터가 20~30개 늘어난다면 코드가 죽 늘어날 수 밖에 없는 것은 마찬가지다
final var taste = Taste.builder()
.like("쌈밥", "김치찌개", "탕수육", "비빔밥")
.dislike("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스")
.build();
final var neo = new Crew(
"neo",
new Balance(10_000),
taste
);- 다음처럼 의미를 더 쪼개서 분리할 수도 있다.
- 명확하게 하는 것은 좋으나. 너무 과하게 설계하면 독이 될 수 있다.
- 본인의 상황에 맞게 좋은 판단을 내리는 것이 중요하다
API를 적절하게 사용하라
class Menus {
public Menus(final List<String> menus) {
for (int i = 0; i < menus.size(); i++) {
for (int j = 0; j < i; j++) {
if (menus.get(i).equals(menus.get(j))) {
throw new IllegalArgumentException("메뉴는 중복될 수 없습니다.");
}
}
}
}
}class Menus {
public Menus(final List<String> menus) {
if (menus.size() != menus.stream().distinct().count()) {
throw new IllegalArgumentException("메뉴는 중복될 수 없습니다.");
}
}
}- 항상 api를 사용할 필요는 없지만, stream 등 준비된 api를 적절히 사용함면 견고한 코드를 작성할 수 있다.
class Menus {
public Menus(final Set<String> menus) {
}
}- Set, List 같은 만들어진 자료구조도 적절히 사용할 필요가 있다
Optional<String> value = map.entrySet()
.stream()
.filter(e -> e.getKey().equals(key))
.map(Entry::getValue)
.findFirst();
- 기능 구현을 위해서 api를 쓰는 것이 아닌, 단순히 stream을 사용하고 싶어서 쓰는 등 ,api를 위한 코드를 작성하는 경우가 있다. 이러지는 말자. 스트림의 저주에 빠지지 말자...
예측 가능한 코드
의미있는 값을 반환하라
public class RacingGame {
private List<Car> participants;
Position averagePosition() {
final var average = participants.stream()
.mapToInt(Car::position)
.average()
.orElse(-1); // 게임 참여자가 없다.
return new Position(average);
}
}- 위 코드에서 -1은 어떠한 희미를 갖고 있는가?
- 읽는 사람이 코드를 예측하는 비용이 소모된다.
public class RacingGame {
private List<Car> participants;
Optional<Position> averagePosition() {
return participants.stream()
.mapToInt(Car::position)
.average()
.stream()
.map(Position::new)
.findAny();
}
}- 그냥 Optional로 반환하여 외부에서 처리하도록 할 수도 있다.
public class RacingGame {
private List<Car> participants;
Position averagePosition() {
final var average = participants.stream()
.mapToInt(Car::position)
.average()
.orElseThrow(() -> new IllegalStateException("게임 참여자가 없습니다."))
return new Position(average);
}
}-
예외를 발생하여 명시적으로 처리할 수도 있다
-
개발을 하다보면 모든 코드에서 null 체크나 예외처리를 할 수는 없다. 그렇기에 네이밍을 통해 의도를 드러내는 것이 중요하다.
public class RacingGame {
private List<Car> participants;
Position averagePosition() {
return participants.stream()
.mapToInt(Car::position)
.average()
.stream()
.map(Position::new)
.findAny()
.orElseGet(Position::ready);
}
}
- Collection은 null을 사용하지 않고 값이 없으면 비어있는 Collection을 반환하므로 힌트를 얻을 수 있다.
public class RacingGame {
private List<Car> participants;
Position averagePosition() {
return participants.stream()
.mapToInt(Car::position)
.average()
.stream()
.map(Position::new)
.findAny()
.orElseGet(Position::ready);
}
}
- 객체 또한 null Object를 사용할 수 있다
- 아직 경주를 시작하지 않은 상황도 하나의 객체로 볼 수 있다
class Car {
private int position;
int move(final int power) {
if (power < 4) {
return position;
}
return position++;
}
}// 조회
final var position = car.move(0);- 명령과 조회를 같은 메소드에서 하고 있다. 따라서 조회 시에 필요하지도 않은 명령을 할 수 있다. 둘 중 하나의 기능만 변경 되어도 서로에게 영향을 준다.
- 그러므로 조회와 명령을 구분해 부수효과를 없애자
중요한 입력에 대해 무시하지 마라
public class CrewMenuRecommendation {
private final Crew crew;
private final List<String> recommendedMenus;
public boolean canRecommendMenu(String menu) {
if (crew.dislikeMenu(menu)) {
return false;
}
if (recommendedMenus.contains(menu)) {
return false;
}
return true;
}
public void recommendMenu(String menu) {
if (cannotRecommendMenu(menu)) {
return;
}
recommendedMenus.add(menu);
}
}- recommendMenu 메소드를 보면 추천할 수 없는 경우에 대해(cannotRecommentMenu) 무시를 하고 있다(리턴값이 없음). 이러면 누군가에게 추천 메뉴가 빈 공간으로 나오는 버그가 나올 수 있다.(나는 추천을 받았는데 추천이 되지 않음)
- 이 경우 확실한 예외처리를 통해 당신이 올바르지 않은 값을 입력했음을 말해주자
열것값을 암묵적으로 처리하지 마라
enum Command {
RETRY,
QUIT;
}public void run(final Command command) {
if (command == RETRY) {
retry();
}
if (command == QUIT) {
quit();
}
}enum Command {
RETRY,
QUIT,
STATUS;
}- 열것값은 언제든지 추가될 수 있다. 다음처럼 열것값이 추가되면 놓친 부분에 대해 수정을 해야 한다
public void run(final Command command) {
if (command == RETRY) {
retry();
return;
}
if (command == QUIT) {
quit();
return;
}
throw new UnsupportedOperationException();
}@Test
void testRun_allCommand() {
for (final var command : Command.values()) {
run(command);
}
}- 예외처리를 확실히 하여 놓친 부분을 확인하자
public void run(final Command command) {
switch (command) {
case RETRY -> {
retry();
return;
}
case QUIT -> {
quit();
return;
}
}
throw new UnsupportedOperationException();
}- switch를 하용하면 컴파일 타임에 경고를 받을 수 있다. 컴파일 타임에 경고를 받으면 베스트다. 그러나 switch/case문은 최대한 쓰지 말라하는데 이 부분은 고민을 해 보자
열거형 사용 방식을 명확히 하라
public enum BridgeGameResult {
UP_RIGHT(true, true) {
@Override
public boolean match(Position position, String tile) {
return position == Position.UP && position.equalPosition(tile);
}
},
UP_WRONG(true, false) {
@Override
public boolean match(Position position, String tile) {
return position == Position.UP && position.notEqualPosition(tile);
}
},
DOWN_RIGHT(false, true) {
@Override
public boolean match(Position position, String tile) {
return position == Position.DOWN && position.equalPosition(tile);
}
},
DOWN_WRONG(false, false) {
@Override
public boolean match(Position position, String tile) {
return position == Position.DOWN && position.notEqualPosition(tile);
}
};
private final boolean isPositionUp;
private final boolean isRightMove;
BridgeGameResult(boolean isPositionUp, boolean isRightMove) {
this.isPositionUp = isPositionUp;
this.isRightMove = isRightMove;
}
protected abstract boolean match(Position position, String tile);
}- 열거 값 자체가 객체로 어떤 기능을 가질 수도 있고, 또 상수로서만 존재할 수도 있다
- 회사나 요구사항에 따라 코드의 작성과 사용이 다르다
원시값을 사용하라
class Position {
private int value;
public void increase() {
value++;
}
public Position decrease() {
value--;
}
}class Position {
private final int value;
public Position increase() {
return new Position(value + 1);
}
public Position decrease() {
return new Position(value - 1);
}
}- 원시값 포장을 하면 의도 전달이 쉽고, 잘못 사용하는 것이 어렵다
- 불변하다면 상태의 변화에서도 자유롭다
변경 가능성을 최소화하라
List<String> excludeDislikeMenus(final List<String> menus) {
menus.removeAll(dislikeMenus);
return menus;
}List<String> excludeDislikeMenus(final List<String> menus) {
final var excluded = new ArrayList<>(menus);
excluded.removeAll(dislikeMenus);
return excluded;
}- 매개변수를 변경할 때 생기는 버그, 그리고 성능 중 무엇이 중요한지 고민하자
List<String> dislikeMenus() {
return dislikeMenus;
}
final var crew = new Crew("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스");
final List<String> dislikeMenus = crew.dislikeMenus(); // ["나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스"]
dislikeMenus.clear();
crew.dislikeMenus(); // []List<String> dislikeMenus() {
return new ArrayList<>(dislikeMenus);
}
final var crew = new Crew("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스");
final List<String> dislikeMenus = crew.dislikeMenus(); // ["나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스"]
dislikeMenus.clear(); // []
crew.dislikeMenus(); // ["나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스"]
- 객체 값을 반환한때 dislikeMenus를 그대로 반환하는 것이 아닌 ArrayList로 복사하는 것 처럼 한번 방어적 복사로 반환하는 것이 안전하다
List<String> dislikeMenus() {
return Collections.unmodifiableList(dislikeMenus);
}
final var crew = new Crew("나시고렝", "파인애플 볶음밥", "미소시루", "하이라이스");
final List<String> dislikeMenus = crew.dislikeMenus();
dislikeMenus.clear(); // 예외 발생- 다음과 같이 외부에서 조작하면 예외가 발생하도록 코드를 짜면 더 안전하다.
생각할 점
- 가독성 높은 코드를 작성하기 위해 어떠한 노력을 해봤는가?
- 예측 가능한 코드를 작성하기 위해 어떠한 노력을 해봤는가?
- 실수를 방지하는 코드를 작성하기 위해 어떠한 노력을 해봤는가?
+) 검증이 필요하지만 값만 반환하는 경우도 원시값 포장이 필요한가?
하마드