[우테코] 7기 프리코스 2주차 회고

 

2주차 미션을 제출한 다음 날, 처음으로 대면 코드 리뷰 스터디를 진행해보았다!
대면으로 하는 스터디는 처음이라 어떻게 진행해야 할지 걱정이 앞서 전 날 진행 순서를 혼자 노트에 정리했다. (ㅋㅋ)
다행히 좋은 스터디원분들과 의미 있는 시간을 보낸 것 같다.
한 번에 여러 명에게 직접 피드백을 듣고, 바로바로 의견을 나눌 수 있는 점이 좋았다.
또, 똑같은 미션인데 서로 다른 방법으로 접근한게 결과물에서도 느껴져서 신기했다.

 

 

1주차 공통 피드백과 개인적으로 받았던 피드백들을 정리하여, 2주차 미션 진행 시 꼭 지켜야 할 주의사항들에 대해 정리한 후 설계를 시작했다.

 

 

2주차 미션 진행 시 주의사항 📑

1주차 공통 피드백

1. 요구사항 준수
2. .gitignore 활용
3. 의미있는 커밋 메세지 사용
4. 오류 찾기 : 출력 대신 디버깅
5. 의도에 맞는 이름 짓기 (축약X)
6. 의미있는 공백 활용
7. 주석 사용 지양
8. Java API 활용
9. 배열 대신 컬렉션 사용

 

1주차 미션 코드 리뷰 피드백

1. AppConfig 활용
2. 재사용되지 않는 변수 선언X
3. 역할 분리
4. else 사용 지양
5. 테스트 코드 꼼꼼히 작성

 

 

잘한 점과 아쉬운 점 🧐

잘한 점

• 의미가 확실하게 전달되도록 네이밍하였다.
• 불필요한 주석을 사용하지 않았다.
• 배열 대신 컬렉션을 적극 사용하였다.
• MVC 패턴을 준수하였다.
• AppConfig를 활용하였다.
• else를 사용하지 않았다.
• 테스트 코드를 최대한 꼼꼼하게 작성하였다.

 

아쉬운 점

• AppConfig에 대한 공부가 부족하다. (팩토리 패턴에 대해서도 공부할 것)
• 일급 컬렉션에 대해 제대로 알지 못한다. (3주차에는 제대로 공부한 후 적용할 예정)
• 결과 출력 시 깊은 복사를 하지 않았다. (제일 바보같았다)
• 재사용되지 않을 변수들을 따로 선언하지 않다보니 가독성이 오히려 떨어진다는 느낌을 받았다.
• 에러 메세지를 따로 관리하지 않고 바로 작성하는 것이 가독성 면에서 좋다는 의견을 수용하여 String으로 바로 작성하였지만, 테스트 시 번거롭다고 느꼈다.
• "실행 결과"가 한 번만 출력된다는 것을 마지막 날 깨달아 급하게 수정하였다.
• 컨트롤러에 너무 많은 의존성이 몰려 복잡해보인다. (리팩토링하며 서비스 레이어 추가)

 

 

리팩토링

리팩토링한 코드는 아래 링크에서 볼 수 있다.

GitHub - IM-GYURI/java-racingcar-7

 

GameService 추가

GameController에 너무 많은 의존성이 몰려 복잡하다는 피드백을 받은 후 GameService를 추가하였다.

기존의 GameController는 InputView, OutputView, CarMaker, RacingGame, RoundNumberValidator를 의존하였지만, GameService를 도입한 후에는 InputView, OutputView, GameService만을 의존하게 되었다. 확실히 깔끔해졌다.

 

 

변수 추가 선언

재사용되지 않는 변수를 선언하지 않으려 한 것이 오히려 가독성을 저해하는 요인이 되었다.

여러 부분을 수정하였지만, 하나만 예를 들어 보겠다.

 

기존의 코드이다.

FinalResultDto finalResultDto = racingGame.playRacingGame(roundNumber, carMaker.makeCars(carNames));

 

아래는 기존의 코드를 수정한 것이다.

List<Car> carList = gameService.createCars(carNames);
FinalResultDto finalResultDto = gameService.runGame(roundNumber, carList);

 

여전히 가독성과 메모리 절약 중 고민이 된다.

 

 

정규식 수정

자동차 이름을 검증할 때, 제어 문자와 메타 문자는 사용하지 못하도록 하기 위해 다음과 같은 정규식을 작성했었다.

    String regex = "[\\p{Cntrl}!@#$%^&*()\\-+=\\{\\}\\[\\]|:;\"'<>?,/\\\\`~]+";

 

하지만 보다시피 변수명도 regex이고, 한 번에 읽히지 않는 정규식이다.

그래서 리팩토링하며 영문 알파벳, 숫자, 한글만 사용 가능하도록 수정해보았다.

 

    private static final String alphaDigitKoreanRegex = "^[a-zA-Z0-9가-힣]+$";

 

변수명도 alphaDigitKoreanRegex으로 만들어 알파벳, 숫자, 한글 정규식이라는 점을 나타내보았다.

 

 

줄바꿈 시 \n 지양

[Java] 개행문자, 줄바꿈 System.lineSeparator()

System.lineSeparator()를 알고 있었지만, \n을 지양해야 하는 이유에 대해서는 처음 알았다.

\n이 사용되던 부분들을 수정하였다.

 

 

추가

@pug9483님께서 남겨주신 싱글톤 관련 내용이 너무 좋아서 공유한다.

 

Singleton을 만드는 방법은 여러가지가 있습니다. 코드 리뷰하다가 내용 수정이 필요해보여서 급하게 작성해봅니다.

방법 1. 보통 사람들이 작성하는 방법입니다. 이 코드는 다중쓰레드 관련 이슈가 있어서 사용하지 않는 것이 좋습니다.

public class AppConfig{
    private static AppConfig unique;    

    public static AppConfig getInstance(){
        if(unique == null) unique = new AppConfig();
        return unique;
     }
}

 

방법 2. synchronized을 통한 다중쓰레드 문제 해결
: 내부적으로 잠금 기술을 사용하기 때문에 추가 비용이 발생합니다. 처음 생성 시에만 동기화가 필요한데 getInstance()를 부를 때마다 동기화를 하니 불필요한 기다림이 생길 수 있습니다.

public class AppConfig{
    private static AppConfig unique;    

    public static synchronized AppConfig getInstance(){
        if(unique == null) unique = new AppConfig();
        return unique;
     }
}

 

방법 3. 동기화 블록과 volatile을 통한 위의 문제 해결
: 처음 객체 생성 시에만 잠금 기술을 사용하기 때문에 2의 문제점이 사라집니다. 하지만 번거롭습니다.

public class AppConfig{
    private static AppConfig unique;    

    public static AppConfig getInstance(){
        if(unique==null){
            synchronized(AppConfig.class){ // 처음 객체 생성 시만 synchronized 사용
                if(unique==null) unique = new AppConfig();
            }
        }
        return unique;
    }
}

 

방법 4. eager instantiation
작성자분께서 사용하신 방법입니다. 불필요하게 일찍 생성되어 자원낭비가 있을 수 있습니다. 하지만, final 키워드도 사용 가능하고 간단하기 때문에 좋습니다.

public class AppConfig{
    public static final AppConfig = new AppConfig();
    public static AppConfig getInstance(){return unique;}    
}

 

방법 5. enum 사용
: 다중쓰레드 문제를 자동으로 해결해줍니다. 권장되는 방법이죠. 하지만, class가 아니기 때문에 유연성이 떨어질 수 있습니다.

 

방법 6. 내부 클래스 사용
: lazy 문제와 동기화 문제가 모두 해결됩니다.

public class AppConfig{
    public static class AppConfigHolder{
        private static AppConfig unique = new AppConfig();
    }

    Public static AppConfig getInstance(){
        return AppConfigHolder.unique;
    }
}

 

메모장에 쓰는 거라 코드상으로 돌아가지 않을 수도 있지만, 보통 이런식으로 다양한 싱글턴 객체를 만드는 방법이 있습니다. 하지만, 싱글턴은 매우 조심해서 사용하셔야 됩니다. 기본적으로 전역 변수의 성격을 띠기 때문에 테스트 코드를 작성하기도 굉장히 번거롭습니다.
따라서, 저는 eager instantiation 방법으로 작성하는 것이 나빠보이지 않습니다.