[우테코 7기 프리코스 1주 차] 피드백

 

2024.10.21 - [우아한테크코스] - [우테코 7기 프리코스 1주 차] 객체 지향

[우테코 7기 프리코스 1주 차] 객체 지향

 

드디어 화요일, 1주 차 코드를 서로 리뷰할 수 있게 됐다! 새로운 과제도 나왔고..

 

리뷰 내용을 바탕으로 배우고 느낀 점을 기록하려고 한다. (+ 백엔드 공통 피드백까지)

 

---

 

indent depth < 3

 

수정 전

public class NumberService {
    public List<BigDecimal> processNumbers(List<String> numberStrings) {
        return numberStrings.stream()
                .map(numString -> {
                    try {
                        if (numString.isEmpty()) {
                            return BigDecimal.ZERO;
                        }

                        if (numString.length() > 1 && numString.charAt(0) == '0' && numString.charAt(1) != '.') {
                            throw new IllegalArgumentException("숫자에 불필요한 0을 포함하지 않습니다.");
                        }

                        BigDecimal number = new BigDecimal(numString);

                        if (number.compareTo(BigDecimal.ZERO) <= 0) {
                            throw new IllegalArgumentException("음수나 0은 허용되지 않습니다.");
                        }

                        return number;
                    } catch (NumberFormatException e) {
                        throw new IllegalArgumentException("잘못된 숫자 형식입니다.");
                    }
                })
                .toList();
    }


    public BigDecimal calculateSum(List<BigDecimal> numbers) {
        return numbers.stream().reduce(BigDecimal.ZERO, BigDecimal::add);
    }
}

 

우테코에서 권장하는 indent(들여쓰기) depth는 3 미만이라는 점을 인지하고 있었지만, 로직을 stream으로 변환하면서 의도치 않게 깊이를 초과하게 되었다.

 

하지만 문제의 본질은 단순히 depth가 아니라, 복잡한 로직을 메서드로 추출하지 않고 한 곳에 모아둔 점이다.

 

코드의 가독성과 유지보수성을 높이기 위해서는, 하나의 메서드가 너무 많은 책임을 갖지 않도록 부분 부분을 적절히 분리해야 했다. 수정 전 코드는 메서드 추출을 놓친 상태였다.

 

SOILD의 단일 책임 원칙, 충분히 적용했다고 생각했는데 부족했다.

 

수정 후

public class NumberService {

    public List<BigDecimal> processNumbers(List<String> numberStrings) {
        return numberStrings.stream()
                .map(this::convertToBigDecimal)
                .toList();
    }

    private BigDecimal convertToBigDecimal(String numString) {
        if (numString.isEmpty()) {
            return BigDecimal.ZERO;
        }
        
        validateNumberString(numString);

        BigDecimal number = new BigDecimal(numString);

        if (number.compareTo(BigDecimal.ZERO) <= 0) {
            throw new IllegalArgumentException("음수나 0은 허용되지 않습니다.");
        }

        return number;
    }

    private void validateNumberString(String numString) {
        if (numString.isEmpty()) {
            return;
        }

        if (numString.length() > 1 && numString.charAt(0) == '0' && numString.charAt(1) != '.') {
            throw new IllegalArgumentException("숫자에 불필요한 0을 포함하지 않습니다.");
        }

        try {
            new BigDecimal(numString); // BigDecimal로 변환해보고 유효성 검사
        } catch (NumberFormatException e) {
            throw new IllegalArgumentException("잘못된 숫자 형식입니다.");
        }
    }

    public BigDecimal calculateSum(List<BigDecimal> numbers) {
        return numbers.stream().reduce(BigDecimal.ZERO, BigDecimal::add);
    }
}

 

processNumber 메서드 내부에 있던 로직을 convertToBigDecimal, validateNumberString 메서드로 분리하여 추출했다.

 

이를 통해 각 메서드의 책임이 명확히 조절되고, indent depth도 물 흐르듯 개선됐다.

 

추후 복잡한 조건문을 가독성을 위해 boolean을 반환하는 메서드로 리팩터링 할까 고려 중이다.

 

---

 

자원 해제, 빈 문자열 입력 처리

수정 전

public String getInput() {
    System.out.println("덧셈할 문자열을 입력해 주세요.");
    String input = Console.readLine();
    validateInput(input);
    return input;
}

 

Console 클래스는 우테코에서 제공되며, Scanner 역할을 한다.

이를 사용한 경우, 자원을 제대로 해제하기 위해 반드시 close를 호출해야 했다. 

try-with-resources를 사용할 수 있었지만, 자원을 명시적으로 해제하기 위해 finally 블록을 통해 close를 호출하는 방식을 선택했다.

정정: Console 클래스는 AutoCloseable를 구현하지 않으므로 try-with-resources를 사용할 수 없습니다.

또한, 실제 터미널에서 실행할 때는 빈 문자열 처리에 문제가 없었으나, 테스트 코드에서는 동일하게 작동하지 않아 이를 별도의 메서드로 처리하여 해결했다.

 

수정 후

    public String getInput() {
        try {
            System.out.println("덧셈할 문자열을 입력해 주세요.");
            String input = getSafeInput();

            validateInput(input);

            return input;
        } finally {
            Console.close();
        }
    }

    private String getSafeInput() {
        try {
            return Console.readLine();
        } catch (NoSuchElementException e) {
            return ""; // 빈 문자열로 처리
        }
    }

 

성공적인 리팩토링~ 테스트도 올바르게 통과된다.

 

---

 

불필요한 주석

 

 

누군가는 주석이 필요하다고 하지만, 반대로 주석이 없어도 자연스럽게 읽히는 코드가 더 좋은 코드라고 주장하는 사람들도 많다.

나 역시 주석이 없는 코드가 이상적인 코드라는 생각에 동의하지만, 그 경계를 찾는 과정이 쉽지 않다고 생각한다.

 

내가 작성한 메인 메서드가 그 좋은 예시다.

 

수정 전

public class Application {
    public static void main(String[] args) {
        // 입력과 출력 담당
        IOService ioService = new IOService();

        // 구분자 처리 담당
        DelimiterService delimiterService = new DelimiterService();

        // 숫자 처리 및 계산 담당
        NumberService numberService = new NumberService();

        // Coordinator에 필요한 의존성 주입
        Coordinator coordinator = new Coordinator(ioService, delimiterService, numberService);

        // 프로그램 실행
        coordinator.execute();
    }
}

 

메서드 내에 각 라인마다 역할을 설명하는 주석을 달아 놓았지만, 실제로 코드 자체가 충분히 직관적이라서 주석이 없어도 코드의 목적과 흐름을 쉽게 파악할 수 있었다. 클래스와 메서드 이름만으로도 어떤 작업을 수행하는지 명확히 드러나기 때문이다.

 

결국, 주석은 코드의 의미를 추가적으로 보완해야 하는 경우가 아니라면, 불필요한 중복 설명으로 작용할 수 있다.

주석 대신 의미가 명확한 클래스와 메서드명으로 충분히 의도를 전달할 수 있다면 주석을 줄이는 것이 더 나은 선택일 것이다.

 

수정 후

public class Application {
    public static void main(String[] args) {
        Coordinator coordinator = new Coordinator(new IOService(), new DelimiterService(), new NumberService());
        coordinator.execute();
    }
}

 

충분히 이해할 수 있는 맥락이다.

수정하고 보니 스프링의 IOC가 그립다.

 

---

나머지 변경사항

 

 

위에서 언급한 주요 변경 사항 외에도, 몇 가지 추가적인 코드 수정을 나열하자면 아래와 같다.

• 테스트 코드에 @DisplayName 애노테이션을 적용하여 테스트 가독성을 향상
• 미처 처리하지 못했던 입력 케이스를 테스트에 반영
• 메서드의 파라미터 타입을 수정함으로써 각 메서드의 책임을 명확하게 분리
• 변수 명명 개선

 

---

 

공통 피드백

 

 

리뷰를 받으며, 풀 리퀘스트를 올린채로 코드 수정 후 커밋을 진행했는데 앞으로는 별도의 리포지토리에 포크해서 진행해야겠다.

 

---

 

 

오류를 찾을 때 디버거를 추천해주신다. 평소 출력 함수와 디버거를 혼용해 왔는데 디버거 사용에 초점을 맞춰야겠다.

 

---

 

 

코드 리뷰에서도 제안을 받았던 부분이다.

예시로 구분자를 기준으로 입력 문자열을 line[0], line[1]로 나눴는데, 명명에 조금 더 시간을 투자해야겠다.

 

---

 

 

평소 글을 작성할 때, 깔끔해 보이기 위해 개행을 자주 사용한다.

그 습관이 무섭게도, 코드 작성에서도 자주 공백 라인을 사용하는데 좋은 예시 코드들을 찾으며 신경 써봐야겠다.

 

---

 

 

컬렉션의 다양한 API를 활용하기 위해서 노력하고, 배열 사용을 지양해야겠다.

컬렉션 API도 자주 사용하지 않으면 금세 잊어버리게 되던데 리팩토링을 진행할 때, 컬렉션 API로 전환할 수 있는 케이스가 있는지 검토해야겠다.

 

---

 

추가 학습 자료 (영상)

 

 

우테코에서 추가 학습 자료로 제공해준 영상을 보며 많은 것을 배웠다. 비록 25분 안에 구현해야 한다는 조건은 나와 조금 다르긴 했지만, 요구 사항에 집중하며 코드를 작성하는 흐름이 나와 비슷해서, 마치 나를 대변해 주는 느낌이 들었다.

 

전체적으로 다른 분들의 PR을 확인해 볼 때 MVC 패턴에 초점을 맞추는 모습이 보였다. 이러한 디자인 패턴이 중요하지 않다는 것은 아니다. 하지만 영상에서 강조하듯, 가장 중요한 것은 요구 사항을 충실히 반영하는 것이라는 점에 깊이 공감했다. 문제를 해결하는 스킬은 그 이후에 자연스럽게 따라올 수 있다고 생각한다. 아마도 다른 분들도 문제 해결에 초점을 맞추셨고, 리팩토링 과정에서 문제를 해결하는 방법을 여러 방면으로 개선한 것이라고 생각한다.

 

결국 핵심은 요구 사항에 충실하면서도 복잡함에 빠지지 않는, 간결하고 목적이 뚜렷한 코드 작성이라는 것을 다시 한 번 깨달았다.

 

맥락에서 조금 벗어나지만, 영상에서 상수 처리가 항상 올바른지 고민하시는 것은 정말 공감됐다.

 

---

 

마무리

 

 

과제를 해결하는 과정에서뿐만 아니라, 마친 후에도 많은 고민이 따랐다. 프리코스를 마칠 때까지 이런 고민들이 밀물과 썰물처럼 들어오고 나가는 과정을 반복할 것이라 생각하니 기대가 된다.

 

---

 

깃허브 PR

 

https://github.com/woowacourse-precourse/java-calculator-7/pull/807

[문자열 덧셈 계산기] 조재중 미션 제출합니다. by m-a-king · Pull Request #807 · woowacourse-precourse/java-ca