[우아한 테크코스 7기] 프리코스 2주차 첫번째 회고 - ✏️ 리뷰를 해보자

처음으로 1주차가 끝나고 리뷰 시간이 찾아왔다! 열심히 고민했기에 다른 사람들의 생각은 어떨지 가장 궁금했다.

그래서 2주차의 첫째날은 리뷰로 하루를 통째로 보냈다.

 

다시 짚어보자면, 내가 이번 프리코스에서 목표로 삼은 것은 두가지이다.

 

1. 두려움 극복
2. 나만의 기준 정립하기

 

여기서 나만의 기준을 정립하기 위해서는 내가 세워둔 기준을 이야기해보고, 다른 사람들의 의견을 들어보며 점차 수정해 나가는 방식을 택하겠다고 이야기했었다. 그래서 이번에는 다른 사람들의 코드를 볼 때에도, 나와 어떤 부분에서 다르게 구현했을지를 중점으로 보고 이유를 물었다.

 

피드백을 받았을 때에도, 단순히 답하고 끝내는 것이 아니라 어떻게 생각하는지 내 의견을 설명해보기 위해 노력하고 다시 상대의 의견을 물어봤다.

 

그럼, 내가 다른 동료들과는 다르게 구현한 부분은 어떤 것들이 있었을까?

1. MVC 패턴의 적용

10개 정도의 리뷰를 남겼는데, 1-2개를 제외하고는 전부 MVC 패턴을 적용한 것을 볼 수 있었다.

이번 프리코스 시작 전에 다짐했던 것이 다른 사람들이 적용한다고 해서 따라하지 말자는 것이었다.

명확한 이유를 찾고, 명확한 이유가 없다면 내가 판단해보고 그럼에도 이유가 딱히 없다면 과감히 적용하지 않는 게 맞다고 생각했다.

 

물론 나 역시도 구현 전에 MVC 패턴이 떠오르긴 했다.

그러나 적용하지 않은 이유는 다음과 같다.

1-1. Model 의 필요성

1주차 과제는 단순하다고 생각했기 때문에 특별히 모델로 만들어야 할 부분을 잘 찾지 못했다.

구분자의 경우에는, 데이터를 정제하는 데에 사용되기 때문에 오히려 캡슐화를 하면 안된다고 생각했다.

계산에 사용되는 숫자의 경우에는 자바에서 이미 잘 정의해둔 숫자와 관련된 모델들이 있기에 굳이 내가 다시 정의할 필요가 없다고 생각했다. 숫자와 관련된 추가적인 비지니스 로직이나 검증 로직도 특별히 없었기 때문이다.

1-2. View 의 필요성

이번 과제에서 입출력은 정말 간단했다. 연산 결과를 딱히 변환할 필요도 없었다.

즉, 이번 과제에서 View 를 정의하려면 우아한 테크코스 측에서 제공해준 Console.readLine 을 감싸는 것밖에 할 수 없었던 것이다.

이걸 감싸서 View 로 정의하는 것은 오버 엔지니어링이라는 생각이 들었다.

1-3. Controller 의 필요성

Controller 는 View 와 Model 사이를 중개한다.

앞서서 계속 이야기했듯, View 와 Model 의 필요성을 느끼지 못하니 Controller 역시도 자연스레 필요 없다는 생각이 들었다.

🤔 그래서, 계속 안쓸 것인가?

내가 받았던 피드백은, 이번 과제의 규모에서는 MVC 를 적용하지 않아도 될 것으로 보이나 추후 변경되는 사항들이 생겼을 때 대응하기 어려울 것이라는 것이다. 또한, MVC 패턴은 널리 사용되고 있으며 유지보수 측면에서 장점을 지니기에 이 패턴에 맞게 설계하는 것이 좋을 것 같다는 이야기였다. 

public static void main(String[] args) {
    System.out.println("덧셈할 문자열을 입력해 주세요.");
    String input = Console.readLine();

    Processor processor = new Processor(input);
    int result = processor.execute();
    System.out.println("결과 : " + result);
}

나 역시도 이에 동의했다. 멀리 볼 것도 없이, 출력 로직이 조금만 복잡해지게 되면 현재 코드는 유지보수 측면에서 단점을 갖는다.

출력이 복잡해지게 된다면 이에 대한 포매팅을 Application.java 에서 해야 하기에 너무 많은 책임을 지게 된다.

또한, 테스트가 추가 된다고 했을 때에도 Application.java 에서 출력 형태를 테스트하기 위해서는 main 메서드를 동작시켜야 하기에 테스트를 위해 입력부터 모든 흐름을 테스트해야 하는 것이다. 이렇게 되면 어디서 왜 오류가 나는지 찾을 수가 없게 되어 버린다.

 

그래서 다음 미션부터는 MVC 패턴을 적용해 보다 변경에 유연한 설계를 해야겠다고 생각을 했다!

2. ErrorMessage 의 상수화

많은 분들이 에러 메시지를 상수화한 것을 볼 수 있었다. 나는 굳이 상수화하지 않았었는데, 예외 처리가 두 군데에서밖에 되지 않기도 했고 상수로 정의한다고 하더라도 재사용되지 않았기 때문이다. 

또한 상수로 정의하지 않고 에러 메시지를 그대로 문자열로 적는다 해도 가독성을 해친다는 생각이 들지 않았다.

 

그러나 코드를 읽는 사람의 입장에서, 에러 메시지가 한 군데에 모아져 있는 것이 장점이 있다는 생각이 들었다.

 

public class ErrorMessages {
    public static final String NUMBER_SHOULD_BE_POSITIVE = "양수만 입력하실 수 있습니다.";
    public static final String LENGTH_OF_NAME_SHOULD_BE_LESS_THEN_FIVE = "이름은 5자 이하여야 합니다.";
}

위는 과제와는 전혀 상관이 없는 예시 코드이다.

코드를 읽는 사람의 입장에서 ErrorMessages 를 쭉 읽기만 해도, 가이드라인을 읽는 것처럼 비지니스 로직이 읽히는 느낌이 들었다.

위의 상수를 읽기만 해도 숫자는 무조건 양수여야 하고, 이름의 길이는 5자 이하여야 한다는 것이 직관적으로 다가온다.

 

또한, 앞서 재사용되지 않았기에 유지보수에 용이하지 않았다는 생각이 들었다고 했는데 이 역시도 추후의 변경 가능성을 고려하지 않았던 부분이라는 생각을 했다. 

 

만약, 테스트가 추가되어 에러 메시지 검증을 한다면? 재사용하게 될 것이다.

UX 라이팅을 다시 하게 되어 모든 에러 메시지의 어투를 변경하게 된다면? 한 군데에 모아져 있는 것이 장점으로 작용할 것이다.

 

이렇게 변경에 언제든지 용이할 수 있도록 고려하는 눈을 키워야겠다는 생각을 했다.

---

💥 개선해 나갈 점

if (input == null || input.isEmpty()) {
	return 0;
}

"" 가 입력값으로 들어오는 경우에는 0 을 반환했어야 하는데, "   " 에는 대응되지 못하는 실수를 범했다. isBlank 와 isEmpty 를 적절히 잘 사용해야겠다는 생각을 했다.

 

다른 분들은 엣지 케이스에 대한 고려를 많이 하신 게 눈에 보였다. 구분자가 두자 이상이라면? 구분자가 숫자로 들어온다면? 등등..

다음 과제를 할 때는 다양한 케이스를 깊이 고려해보거나, 정의되지 않는 공백들을 직접 촘촘히 채워 나가야겠다는 생각을 했다.

 

위에서 이야기한 isBlank 를 사용하지 못하고 놓친 부분도, 엣지케이스에 대해 조금 더 깊이 고려했다면 분명 잡았을 부분이라고 생각해 아쉬움이 든다. 이번 과제에서는 테스트 코드를 작성하게 되니, 조금 더 촘촘히 잡아낼 수 있지 않을까 싶다!

---

추가적으로, 이번 과제의 리뷰를 하며 다음 과제에서는 적용시켜 보고 싶은 것들이 있었다.

1. 팩토리 패턴

MVC 패턴을 사용하신 분들 중에, 컨트롤러에 뷰와 모델을 주입하기 위해 팩토리 패턴을 사용하신 분을 봤다.

스프링부트와 같은 프레임워크에서는 이와 같은 주입을 개발자가 할 필요가 없기에 이런 생각은 하지 못했는데 이번 과제에서는 이를 적용시켜 봐야겠다는 생각을 했다.

2. 부정을 없애라

public static void isAllNumbers(String[] splitInput) {
        for (String token : splitInput) {
            if (!isNumeric(token)) {
                throw new IllegalArgumentException("구분자 이외의 문자열이 포함되어 있습니다.");
            }
        }
    }

public static void isAllNumbers(String[] splitInput) {
        for (String token : splitInput) {
            if (isNumeric(token)) {
                continue;
            }
            throw new IllegalArgumentException("구분자 이외의 문자열이 포함되어 있습니다.");
        }
  }

나는 첫번째와 같은 코드를 작성했었다. 그러나 부정어를 사용하지 않는 것이 좋겠다는 피드백을 받았다. 이에 대해 평소에 알고 있었지만, 놓쳤다는 생각이 드는 동시에 왜 부정어를 사용하지 않는 게 좋을까? 하는 생각을 다시 하게 됐다.

마침 다른 분께서 유효성 검사 로직에서는 부정어를 사용하는 것이 오히려 직관적인 것 같다는 피드백을 주셔서 내 생각을 한번 정리해봤다.

부정어를 사용하는 경우

코드를 읽는 사람 입장에서 숫자인지 확인하는구나 -> 그런데 부정 연산자가 있네 -> 숫자가 아닌 경우는 어떤 것들이 있지 이렇게 한번 직접 뒤집어서 생각해야 하기 때문에 사고에 조금 더 많은 시간을 쏟아야 한다고 생각했다.

  public static void isAllNumbers(String[] splitInput) {
        for (String token : splitInput) {
            if (!isNumeric(token)) {
                throw new IllegalArgumentException("구분자 이외의 문자열이 포함되어 있습니다.");
            }else{
                  // do something...
             }
        }
    }

만약 위와 같이 else 문이 있다면 뒤집은 사고를 또 다시 뒤집어 그 이외의 상황들을 떠올려 봐야 한다.

부정어를 사용하지 않는 경우

숫자인지 확인하는구나 -> 숫자인 continue 구나 숫자면 유효한가보다 이렇게 사고의 흐름이 단순해진다고 생각했다.

  public static void isAllNumbers(String[] splitInput) {
        for (String token : splitInput) {
            if (isNumeric(token)) {
                continue;
            }else{
                // do something..
             }
        }
    }

이 역시도 else 문이 추가되더라도 else 문 이하는 숫자가 아닌 경우구나 하고 비교적 사고의 흐름이 간단해질 것으로 생각했다.

 

이렇게 한번 말로 나의 의견을 정리해보니 더 좋은 기회가 되었다!

---

🥹 더 나은 리뷰어가 되고 싶어

https://tech.kakao.com/posts/498

효과적인 코드리뷰를 위한 리뷰어의 자세 - tech.kakao.com

 

오늘 하루를 리뷰로 보내며 리뷰의 좋은 점들을 속속 골라 먹었다는 생각이 들었다. 나의 의견을 설명하며 나 스스로도 정리해보고, 내가 궁금했거나 확신이 서지 않았던 부분들을 질문도 해보고. 이와 동시에 더 효과적인 피드백을 전달하기 위해 더 나은 리뷰어가 되고 싶다는 생각을 하게 됐다.

 

다음 주차 리뷰를 할 때에는 위의 이야기를 참고해 리뷰를 해봐야겠다!

---

저번 주차 회고를 하며, 이번 주차에는 꼭 나의 의견을 공유해보고 싶다고 이야기를 했었는데 불과 하루만에 어느 정도 목표를 이룬 것 같아 뿌듯했다! 앞으로도 과제가 마무리 되고 하루 정도는 리뷰를 위한 시간으로 남겨둬야겠다는 생각을 했다.

타인의 의견을 적대적이지 않고 유연하면서도 내 의견과 중심을 지키며 받아들이는 연습을 하고 있다는 생각을 했다.

 

남은 날들도 파이팅!