다른 사람과 내 코드를 비교해보면서 그분의 부족했던 점은 무엇인지, 나의 부족한 점이 무엇인지 확인하여 학습하고자 코드 리뷰를 했다.
코드 리뷰를 했을 때 답변해주는 사람이 있는 반면 답변을 못한 사람도 있었다. 하지만 코드 리뷰는 상대방의 코드의 부족한 점을 짚어주는 목적도 있지만 상대의 좋은 점을 칭찬하고 흡수하는 목적도 있기 때문에 답변을 못해줘도 그들의 스킬을 전수받으며 내가 성장하는 시간이었다고 믿는다.
그리고 내가 내 코드를 보면서 부족한 점이 무엇이지 찾는 것보다 상대방이 찾는 것이 훨신 효율적이라고 생각한다. 내가 열심히 짠 코드이고 나는 사람이기 때문에 완전히 객관적으로 볼 수 없기 때문이다.
내가 코드 리뷰를 받으려면 다른 사람의 코드 리뷰를 해줘라는 제이슨님의 조언으로 나는 남는 시간이 있을 때 한번씩 코드 리뷰를 진행한다. 언젠가 코드 리뷰를 주고 받는 한국의 정을 느껴보고 싶다.
따라서 "1일 1커밋"처럼 "1일 1코드리뷰"를 하려고 한다.
황당했던 일이 있었는데 코드리뷰를 다 해놓고 완료를 누르지 않아서 상대방에게 보이지 않았던 일이 있었다. 몰랐던 나는 코드 리뷰를 했는데 다들 답변이 없어서 침울해 있었다. 리뷰 완성 버튼을 누르니까 바로 답변들이 오더라
코드 리뷰를 하고 꼭 "complete review"를 누르자..
이런 리뷰를 보면 기분이 좋아진다.
2주차 과제에 들어가면서 1주차보다 처음부터 기능을 체계적으로 정리를 하고 싶었다. 1주차 때는 Number 클래스를 나중에서야 추가했기 때문에 리팩토링을 진행할 때 굉장히 애를 먹었던 경험이 있다.
이런 경험을 극복하고자 게임 진행을 플로우 차트로 표현을 해보았다. 플로우 차트의 각 항목은 메서드의 이름이 될 수 있다.
그리고 이 플로우 차트를 클래스 별로 나누려고도 해봤다. 관련된 메서드끼리 모아서 하나의 클래스의 메서드라고 표현했다. 아래 사진은 클래스를 색깔별로 나눈것이다.
메서드가 입력 값이 숫자인지 예외처리하는 클래스를 Number가 아닌 입력 클래스에서 해야할 지 고민을 많이 했던 것 같다.
다음으로 Game 클래스의 메서드 별로 플로우를 쪼겠다. 그 이유는 Game는 전체적인 게임의 흐름을 관장하는 클래스이기 때문에 메서드 당 큰 파트로 나눌 수 있을 것 같다는 생각 때문이었다.
이전에는 "선 기능 구현, 후 테스트 코드 작성" 순서로 개발을 진행했고 이런 순서가 당연한 생각이었다. 없는 걸 어떻게 테스트하냐는 논리였다.
이전 주차에서 TDD에 대해 관심이 생겨서 TDD에 대한 내용을 학습했다.
TDD 매커니즘에 대해 완벽하게 배웠다고 자신하지는 못하지만 대략 어떤 느낌인지 감을 잡았다.
TDD의 가장 중요한 아이디어는
1. 먼저 요구사항을 반영하는 테스트 코드를 작성하고
2. 그 테스트 코드를 만족시키는 코드를 작성한다.
3. 성능, 재사용성, 가독성이 좋은 코드를 만들기 위해 리팩토링한다.
@Test
void 자동차_이름_5자초과_예외처리_테스트() {
assertThatThrownBy(() -> new Car("송민준입니다")).isInstanceOf(IllegalArgumentException.class);
}
public class Car {
public Car(String carName) {
this.carName = carName;
this.movingDistance = 0;
}
private void validateCarNameLengthExceed(String carName) {
if (carName.length() > 5) {
throw new IllegalArgumentException("자동차의 이름은 5자를 넘어갈 수 없습니다.");
}
}
public class CarName {
String carName;
public CarName(String carName) {
validateCarNameLengthExceed(carName);
validateCarNameLengthZero(carName);
this.carName = carName;
}
private void validateCarNameLengthExceed(String carName) {
if (carName.length() > 5) {
throw new IllegalArgumentException("자동차의 이름은 5자를 넘어갈 수 없습니다.");
}
}
이렇게 TDD 방식대로 테스트 코드를 먼저 작성해보니까 이전에는 내가 싸질러(?) 놓은 코드들에 대한 테스트 코드를 작성하는게 귀찮았던 반면 함수에 대한 테스트 코드를 먼저 작성하니까 테스트를 성공시키려고 노력하는 나를 발견했다.
그리고 이번 주차의 목표가 "함수의 분리"와 "함수별 테스트 코드 작성"이었는데 TDD 방식으로 개발하기 위해 내가 만들 함수 별로 테스트 코드를 작성하고 코딩을 시작하니까 여러 기능이 하나의 메서드에서 사용되는 일이 거의 없었다. 추가적으로 단위 테스트를 진행하기 위해 하나의함수에서 여러 함수로 쪼개지게 되므로 클래스의 분리가 일어나고 재사용성, 가독성이 좋아지는 결과를 볼 수 있었다.
즉 TDD 방식으로 개발을 행하니 함수별로 테스트 코드를 작성하게 될 수밖에 없었고 그 결과로 함수의 분리를 이뤄낼 수 있었다.
자주 사용하는 메서드를 모아두는 유틸 클래스는 인스턴스가 생성될 일이 없다. 자기 자신의 변수를 가지고 있지 않기 때문이다. 하지만 코드를 그대로 두면 생성이 가능하다는 사실. 생성되면 쓸데없이 메모리만 낭비할 뿐이다.
public class Dice {
private Dice() {}
public static boolean isSuccess() {
return (getRandomNumber() >= 4);
}
}
이는 Dice 유틸 클래스이다. 이렇게 생성자를 private 접근자로 만들면 Dice 인스턴스를 생성할 수 없고 Dice의 기능을 사용하기 위해 isSuccess()
사용해야한다.
따라서 이 클래스의 생성자의 의도 했던 대로 사용자는 클래스를 생성하지 않을 것이다.
시도 횟수 입력을 받아야하는 상황에 입력이 정수를 입력했는지 아닌지 예외를 처리하는 요구상황이 있기 때문에 문자가 숫자인지 확인하는 방법을 서칭했다.
가장 간단한 방법은 Integer.parseInt
를 이용하는 방법이다.
public int convertTrialNumberInputToInteger(String trialNumberInput) {
try {
int trialNumber = Integer.parseInt(inputString);
} catch (NumberFormatException ex) {
throw new IlligalArgumentException();
}
return trialNumber;
}
parseInt
를 실행하면 문자열이 정수인지 판단해, 아니라면 NumberFormatException을 수행할 수 있다.
하지만 여기서 다른 문제점이 있었는데 위의 방식으로 하면 간단하지만 하나의 메서드가 두개의 기능을 수행함을 발견했기 때문이다.
따라서 다른 방법을 이용하기로 했다.
기본적으로 String 클래스는 match()
라는 정규표현식을 준수하는지 검사하고 true/false를 리턴하는 메서드가 있다. 정규 표현식을 사용하면 정수인지 확인할 수 있다.
[+-]?
: + 또는 - 중 "한개"가 있어야한다. (만약 1주차 과제라면 "1", "2"가 아닌지 구분하기 위해 정규표현식으로 restartNumber.matches("[12]?")
를 사용했을 것이다. )\\d*
: "한 개 이상"의 정수가 있어야한다. \\d+
: 한 개 이상의 정수를 "허용"한다. public int convertTrialNumberInputToInteger(String trialNumberInput) {
validateTrialNumberNotInteger(trialNumberInput);
return Integer.parseInt(trialNumberInput);
}
public void validateTrialNumberNotInteger(String trialNumber) {
if (trialNumber.matches("\\d*")) {
return;
}
throw new IllegalArgumentException("시도 횟수는 정수를 입력해주세요.");
}
이를 내 코드에 적용시켰다. 성공적으로 기능을 분리할 수 있었고 요구사항을 만족할 수 있었다. 이전에 비해서 convertTrialNumberInputToInteger
의 가독성이 좋아졌다.
커밋 메시지를 작성할 때마다 헷갈리는 부분이 있다. Scope를 정의하는 것이다.
다음과 같이 3개의 변경사항이 존재한다. 2주차 깃 요구사항에 기능 단위로 커밋을 수행하라는 메시지가 있기 때문에 커밋 제목을 기능으로 넣으려고 노력했다.
커밋을 기능단위로 하면 위와 같이 하나의 클래스만 변경되지 않는다. 그 기능을 수행하는 메서드를 가진 클래스, 그리고 그 클래스를 사용하는 클래스 두 종류의 클래스가 변경이 된다. 그럼 두가지 클래스를 모두 적용해야할까?
원래 생각은 기능을 수행하는 코어적인 클래스만 스코프에 적어야한다고 생각했다. 이런 애매한 상황에서 깃 커밋 스코프에 대한 것을 서칭했다.
적절하다고 생각한 답은 모든 클래스를 적는 것이다. 이름이 들어가면 어디가 변경되었는지 알 수 있기 때문이다. 사이드이펙트로 코드가 변경된 부분도 에러가 일어날 수 있는 부분이기 때문이다.
위 사진에서 적절하게 바꾸면 feat(Car, CarTest): ...
가 될 수 있다.
커밋은 내가 볼 수 있지만 다른 사람도 볼 수 있기 때문에 변경된 점을 명확하게 명시하는게 좋다.
1주차 피드팩에서 "배열 대신 Java Collection을 사용한다"라는 내용의 피드백이 있었다. 피드백을 받고 1주차 때도 컬렉션을 사용하기는 했지만 List를 사용했다면 생성, 삭제같이 기본적인 기능들만 사용했다. 2주차에는 보다 더 적극적으로 활용을 하려고 노력했다.
그 사례로 최종 우승자를 가려내는 기능을 구현한 것을 들 수 있다.
이는 원래 구현할 기능 목록이다.
- [ ] 최종 우승자를 판정하는 기능 구현
- [ ] 자동차별로 이동 거리 확인
- [ ] 최고 이동 거리면
- [ ] 최고 이동거리 갱신
- [ ] 우승자 리스트 clear하고 자동차 추가
이를 구현하기 위해서는 아래와 같이 코드를 짜야했을 것이다.
int maxDistance = 0;
List<Car> winnerList = new ArrayList<>();
for(Car car : cars){
if(car.distance > maxDistance) {
maxDistance = car.distance;
winnerList = new ArrayList<>();
winnerList.add(car);
}
else if(car.distance == car.distance){
winnerList.add(car);
}
}
충분히 잘 돌아갈 수 있는 코드지만 if else 문이 있고 들여쓰기의 depth가 2개 이상이기 때문에 가독성이 떨어질 수 있는 코드다.
다행히도 List 컬렉션은 sort()
라는 좋은 기능이 존재한다. sort()
는 정해진 규칙에 위해서 컬렉션 안의 요소들을 정렬시킨다.
sort
를 사용했기 때문에 변경된 기능 목록이다.
- [ ] 최종 우승자를 판정하는 기능 구현
- [ ] 자동차 이동거리 순으로 정렬
- [ ] 가장 첫번째 자동차와 모든 자동차의 이동거리를 비교
- [ ] 같은 이동 거리면 우승자 리스트에 추가
아래는 Comparable
을 구현하고 compareTo
를 오버라이딩해서 정렬할 때 필요한 규칙을 정하는 Car 클래스다.
public class Car implements Comparable<Car>{
private CarName carName;
private int movingDistance;
...
@Override
public int compareTo(Car o) {
return o.movingDistance - this.movingDistance;
}
public class Winners {
...
private void decide(List<Car> cars) {
cars.sort(Comparator.reverseOrder());
Car firstWinner = cars.get(0);
int carIndex = 0;
while (carIndex < cars.size() && IsNextCarWinner(cars.get(carIndex), firstWinner)) {
winners.add(cars.get(carIndex));
carIndex++;
}
}
...
}
같은 값이 있는지 알려주는 List.contains
메서드다.
List<String> strList = new ArrayList<>();
strList.add("MJ");
strList.contains("MJ"); //true
List<Integer> intList = new ArrayList<>();
intList.add(123);
intList.contains(123); //true
문자열, 숫자와 같은 기본 자료형은 쉽게 리스트에 포함되어 있는지 알 수 있다.
하지만 우리가 정의하는 클래스는 다르다.
List 클래스는 contains를 할 때 "어떤 방식으로 같음을 증명하는지 알아야" contains를 사용할 수 있다.
따라서 equals()
를 오버라이딩 해줘야한다.
private void validateDuplicatedCarName(Car car) {
if (cars.contains(car)) {
throw new IllegalArgumentException("차 이름이 중복되었습니다.");
}
}
다음은 내 프로젝트에서 중복된 자동차 이름을 예외처리하는 메서드다.
물론 getter를 사용해 가져와, 같은지 비교할 수 있다.
하지만 가독성도 떨어지고 캡슐화를 망가뜨리는 원인이 될 것이다. 따라서 List.contains()
를 활용하기로 했다. 활용하기 위해서는 Car의 equals
를 오버라이딩 해야한다.
public class Car {
...
@Override
public boolean equals(Object obj) {
String opponentCarName = ((Car) obj).carName;
return opponentCarName.equals(carName);
}
}
업캐스팅한 뒤에 상대 문자열과 자신의 문자열을 비교해서 같은 지 확인한다. 이로써 List.contains()
를 활용할 수 있게 되었다.
결과적으로 자바 컬렉션의 도움을 받고 기능을 더욱 쉽게 구현할 수 있었다. 기능을 구현할 때마다 자바 컬렉션의 사용을 염두해 둬야겠다는 생각이 들었다. sort 나 contains 와 같이 좋은 메서드가 있을 수 있기 때문이다. 자바 컬렉션은 우리가 필요없는 코딩을 하지 않도록 도와준다.
코드 리뷰를 하면서 상대방 뿐만 아니라 나도 큰 성장을 이뤄낼 수 있음을 알게 되었고 테스트 코드를 먼저 작성했을 때의 설계 상의 이점 등 이번 주차도 많은 것을 학습했던 주였다. 학습에 재미를 느끼는 만큼 다음 주차도 기대가 된다.