[코드 리뷰 이야기] 파동권! 승룡권! 궁극의 체인 콤보를 너에게만 알려주마! (1/3)

• 지난 이야기: [코드 리뷰 이야기] 단수형을 단수형이라 부르지 못하고, 복수형을 복수형이라 부르지 못하니
• 사전 학습 자료: 오리지널 콤보, 슈퍼 콤보, 울트라 콤보

이 글은 지루한 내용을 쉽게 전달하기 위해

저렴한 B급 정서가 의도적으로 weaving 되어 있습니다.

읽기에 불쾌하지 않은 수위를 유지하려 합니다만

혹, 거북하거나 적절치 못한 표현으로 글 읽기에 방해가 된다면

피드백 주시는대로 수위 조절을 하도록 하겠습니다.

참고로 이 글은 읽는 호흡에 맞춰 개행이 되어 있습니다.

화면 폭이 좁은 모바일 기기에서는 개행이 부자연스러울 수 있으니 양해 부탁드립니다.

• 개발 언어: Java
• 버전: 무관
• 정적 분석 도구: PMD (Eclipse Plugin)
• 개발 도구: Eclipse (for Java EE Devlopers, for Java Developers)

---

부제: ↓ ↘ → + 펀치 커맨드로 Violation을 제거하라!

코드 리뷰 두 번째 이야기입니다.

오늘은 그냥 리뷰만 해서 지적질을 하는 것이 아니라

리뷰한 내용을 효율적으로 시정 조치하는 방법에 대해 알아봅니다.

이 글의 주제는 코드 리뷰입니다.

하지만 사실 코드를 들여다보고 상의하고 고치다보면

이게 인스펙션인지, 워크쓰루인지,

피어 리뷰인지, 정적 분석인지 경계가 모호해지곤 합니다.

아마도 각종 품질 프로세스나 성숙도 모델,

혹은 각종 품질 관련 매뉴얼들을 보신 분들은 아시겠지만

사실, 이런 활동들에도 나름의 구분이 있고

리뷰어 구성에서 진행 방법, 형식에 이르기까지

조금씩 상황에 따라  적용하는 형태에 차이가 있습니다.

이러한 리뷰의 분류나 차이점에 대해서는

자격증을 준비하시는 분들에겐 의미가 있겠습니다만

당장의 목표가 문제 해결인 개발자의 입장이라면

현재 내가 하고 있는 리뷰 방식이

세팍타크로를 하고 있는 것인지,

족구하고 있는 것인지 (발음주의)

그리 중요한 것은 아닐 것입니다.

그래서 앞으로 굳이 리뷰의 형태를 구분해야는 경우가 아니라면

특별히 구분하지 않고 '리뷰'로 통틀어서 진행하려고 합니다.

물론 이론 공부도 기술 역량의 펀더멘탈이 되는 중요한 요소이니

무시하자는 것은 아니고, 지금은 리뷰 후, 조치 방법에 대해 집중하기 위해

우선은 양복 자켓 잠깐 벗고 츄리닝으로 갈아 입읍시다.

코드 리뷰로 시작해서 정적 분석 살짝 스친 다음

재발 방지를 위한 룰셋 정의까지 한번에 갑니다.

(룰셋 정의는 다다음 연재로 미룹니다.)

특별히 오늘은

• 코드 리뷰가 좋은 건 다 안다, 교과서는 그만 읽고 실전을 알려달라.

... 라고 보채시는 여러분을 위해

오늘은 궁극의 필살 콤보를 살짝 맛보여 드리겠습니다.

자, 오늘의 떡밥을 공개하겠습니다!

오늘도 어려운 소스 코드를 쓰지 않고

어처구니 없이 간단한 코드로 이야기를 풀겠습니다.

우선 이 코드를 먼저 보겠습니다. (빠밤)

원본 소스

1
String XXX_XXXX = "yyyy xxxx zzzz";

• 에헤이... 아버님, 좀 더 쓰시지, 너무 쉬운 문제를 주셨네요. (10명의 접속자가 퇴장하셨습니다.)

네, 이러려고 개발했나 모욕감이 들 정도의 간단한 코드입니다.

그럼에도 불구하고 리뷰 중에 이런 코드가 발견되면

팀원들 사이에 이런 이야기들이 오고 갑니다.

• 이거 뭔지 알아요. 상수를 final로 선언하란 말이죠? (안상수 보온병 포탄... 드립할까 말까)
• public인지, protected인지, private인지 안적었어요.
• 움... 안적으면 default가 뭐더라? (Java 입문서 뒤적뒤적)
• 상수라고 생각하고 만든거라면 static으로 해야 합니다.
• 그런데.. static final로 써야하나? final static으로 써야하나? (Eclipse 타이핑 토닥토닥)

네, 다들 잘하고 있습니다. 딩동댕동

디테일한 부분은 JavaDoc이나 개발 입문서를 보면 확인이 가능하니까

이 정도의 촉이 살아 있다면 이 친구들과 뭔가 도모 해 볼 만하죠.

(촉촉함이 살아 있는 치킨집을 도모해보자. 도~모 이랏샤이마세!)

간혹, 경험 많은 개발자가 개발팀에 섞여 있다면

이런 이야기를 들을 지도 모릅니다.

• 혹시, 저거.. 상수 아닌거 같은데.. 그것 참 모호하네... (그건 상수가 아니라, 안철수... 드립할까 말까)
• 저 상수... 참조하는 코드가 어떤거죠? 추적 한번 해봅시다.
• 같은 상수가 다른 클래스에도 있네요. 공통으로 Constants 파일을 하나 만들어서 몰아넣죠.
• 그런데 다른 패키지에서도 이 상수를 쓰네요.(그 분은 전 인천시장 안상수... 드립할까 말까)
• Constants 파일의 위치를 좀 더 상위 패키지로  refactoring move할까요?
• 가만... 다른 클래스에서는 이름은 같은데 값이 다른 걸 쓰네요.
• 움... 이런 경우에는 국지적으로 해당 패키지나 클래스 내에 한벌 더 만들어서 그걸 참조하게 합시다.

등등등...

팀원들의 사고하는 폭이 조금 더 넓어진 것이 느껴지시나요?

지금 그들은 해당 변수(혹은 상수)가 재사용되는 범위와 중복 여부를 확인하려 하고,

중복 되었다면 제거하려 하고,

사용 범위를 보고 패키지나 클래스의 위치를 조정하여 리팩토링하려 하고,

같은 이름에 대해서는 패키지 네임 스페이스를 달리주어

간섭이 생기지 않도록 격리를 하려고 하고 있습니다.

아주 훌륭합니다.

정말 현장에서 이런 모습 보면 보람찹니다. (BGM: 팔도 사나이)

우리가 코드 리뷰를 하면서 궁극적으로 원하는 것은

정제된 소스코드이기도 하겠지만

이렇게 코드를 함께 보고 대화하고 개선하려고 하는

자발적인 개선의지와 학습 욕구에 대한 동기부여이기도 합니다.

자, 이제 머리가 좀 말랑해지고 몸이 좀 풀렸다면

이제 실세계로 돌아와서 여러분의 코드를 살펴봅시다.

진짜 세계에 온 것을 환영하네. (Welcome to the Real World.)

자, 이제 앞서 맛본 코드와 비슷한 맥락으로

형상 관리 서버를 뒤져보면 당장이라도 튀어 나올 법한

실세계의 코드를 살펴봅시다.

유형 1: VariableNamingConventions

유형 1: 시술 전

1
private static String XXX_XXXX = "yyyy xxxx zzzz";

자, 아주 쉽습니다.

앞서 살펴본 코드를 리뷰하면서 이미 답이 나온 것이죠.

이런 코드에 PMD라는 정적 분석 툴을 걸어보면

VariableNamingConventions라는 이름의 위반으로 분류합니다.

요컨데 언더스코어('_' 문자)를 쓴 걸 보니

상수라고 쓴 것 같은데, 그렇다면 final을 붙이던지

그게 아니라면 언더스코어를 쓰지 말라는 말입니다.

위 내용이 상수를 쓰려고 했다고 가정하면

위 코드는 아래와 같이 바뀌어야 합니다.

유형 1: 시술 후

1
private static final String XXX_XXXX = "yyyy xxxx zzzz";

네, 깔끔합니다. 전혀 어렵지 않죠.

우리의 QA와 개발자는 이런 훈훈한 대화를 나눕니다.

• QA: 아, 이 위반 건은 조치가 어렵지 않으니 내일 오후까지 수정 부탁해요.
• 개발자: 네, 이 정도는 개발의 땀이죠. 세발의 피죠.

네, 별 것 아닙니다. 그래서 점심 먹고 손봐야지 했는데

오후에 확인해보니 비슷한 패턴으로 위반 사항이 60,000여건이 있습니다.

퇴근 시간까지 4시간 남았으니 4 * 60 = 240분,

60,000건 / 240분 = 250건, 1분에 250건을 처리해야 하고,

250건 / 60초 = 약 4건, 1초에 4건이상을 처리해야 합니다.

미드 24시의 잭바우어가 온다해도 이건 해결 못합니다.

네, 야근 확정이네요. 축하합니다. (분노 게이지가 100 상승하였습니다.)

도움!

큰일났습니다.

제 때 퇴근은 물건너 간 것이고

집에 간장 게장이 택배로 왔는데 빨리 가봐야한다는 드립은

이미 지난 주에 써먹어 재사용이 불가능합니다.

하지만 여긴 회사,

어려움이 있으면 선배님께 즉시 보고합니다.(이럴 때만 선배 찾...)

자초 지종을 들은 선배 PL님

역시 제 때 퇴근하기는 커녕,

내일도 소스 품질 구리다고 PM님께 압박 받을 걸 생각하니 눈 앞이 캄캄합니다.

아무일도 없었던 것처럼 QA와 개발자를 땅에 묻어 버리려니

AI 때문에 더 파묻어버릴 구덩이도 없습니다.

절체 절명의 위기, 이 순간이 생의 마지막이라고 생각하니

지난 날, 아름다웠던 일상의 추억들이 주마등처럼 지나갑니다.

머릿속이 복잡해지고 정신이 혼미해지는 가운데

코딩의 신이 계시를 주십니다.

커피는 레귤러 익스프레션, 션, 션, 션, 션, ... 고용은 정규 표현식, 식, 식, 식, 식, ... 호주 이민은 스칼라, 라, 라, 라, 라, ...

네, 맞습니다. 우리에겐 고대부터 전해져 오는 궁극의 필살기

걸면 걸리는 걸리버, 귀에 걸면 귀걸이, 코에 걸면 코가 걸리는 정규 표현식이 있습니다!

코딩 신의 말씀입니다. 아멘.

코딩계의 몽키 스페너, 정규 표현식

정규 표현식이 대체 뭔가요?

처음 듣는 개발자는 일단 복도로 나가 엎드리시고

남은 분들에게만 설명하겠습니다.

정규 표현식은 보통 입력 값 검증,

특히 이메일 주소나 카드 번호 validation 등에 많이 활용됩니다.

하지만 소스 코드 수정 시에 활용하시는 분들은 그리 많이 본 것 같진 않네요.

정규 표현식은 특정 문자열의 패턴을 찾아서 검증 하는 용도로 사용하지만

찾은 패턴을 치환하는 용도로도 사용할 수 있습니다.

그래서 위와 같이 수정할 대상과 목표가 명확한 경우,

즉 일괄 수정으로 일망타진할 수 있는 이런 경우야 말로

정규 표현식의 힘을 제대로 발휘할 수 있는 때입니다.

그러면 다시 한번 변경해야할 패턴과 변경된 후의 패턴을 살펴보죠.

정규 표현식을 사용할 때의 한가지 요령을 알려드리자면

탐색해야할 패턴의 특징을 살펴본 후,

가변시킬 부분과 가변시키지 않고 그대로 재사용할 부분을 가려내야 합니다.

자.. 실눈을 뜨고 패턴을 떠 올려 봅시다.

유형 1: 시술 전

1
private static String XXX_XXXX = "yyyy xxxx zzzz";

패턴이 떠 오르십니까?

이제 바뀌어야할 패턴도 살펴봅시다.

유형 1: 시술 후

1
private static final String XXX_XXXX = "yyyy xxxx zzzz";

감이 오시나요?

단도직입적으로 결론부터 말씀드리자면

아래와 같은 패턴을 정규 표현식으로 찾고 치환을 해주면 됩니다.

유형 1: 정규 표현식으로 탐색할 패턴 (Find)

1
private static String ([A-Z]{1,}+)

유형 1: 정규 표현식으로 치환할 패턴 (Replace)

1
private static final String $1

오홋, 정규 표현식에 아직 익숙치 않은 분들은 저게 뭔가 싶겠습니다만

문자열의 패턴을 찾고 특정 패턴을 재사용하여 치환하고 있습니다.

참고로 이 포스트는 코드 리뷰를 주제로 하고 있어서

정규 표현식에 대한 자세한 내용은 다루지 않습니다.

(별도의 포스팅을 할지는 여부는 아직 계획 없음)

어쨌거나 위에서 살펴본 violation을 시정 조치하는 방법은 다음과 같습니다.

파일 내에서 치환 방법

1. Ctrl + f  눌러  'Find/Replace'  대화 창을 띄움
2. 'Find: ' 입력 란에 탐색할 패턴 입력 (예: private static String ([A-Z]{1,}+))
3. 'Replace with: ' 입력 란에 치환할 패턴 입력 (예:private static final String $1)
4. 'Options'에서 'Regular expressions' 체크 박스 활성화
5. 'Replace All' 버튼 선택
6. 치환된 결과 확인하여 이상 없으면 'Close' 버튼 선택
7. 일괄 치환 완료
8. 소스 코드 commit

감이 오십니까?

글로만 보면 감 떨어지므로 (까마귀 날자...아.. 그건 배였나?)

이건 꼭 Eclipse에서 실습을 해봅시다.

IntelliJ 사용자는 브루주아는

이미 충분한 혜택을 받고 계시므로 스스로 방법을 찾아서 해결하시면 됩니다.

자, 이렇게 해서 Violation 하나를 처리하였습니다.

밥 값 했네요. 갑자기 배가 고파집니다.

• 아버님, 뭘 이딴 걸 알려주고 생색을 내려 하십니까?

그쵸? 패킷 아깝게...

오늘은 제가 궁극의 콤보를 알려드린다고 했습니다.

그래서 우선은 가볍게 몸을 푸는 콤보를 알려드리죠.

받아랏, 파동권! (오리지널 콤보)

자, 이제 이것을 연속 동작으로 해 봅시다.

참고로 스트리트파이터에서 파동권의 콤보는 아래와 같습니다.

1. ↓ ↘ → + 펀치 커맨드

위와 같은 콤보가 Eclipse에서는 먹힐리 만무하니

위에서 살펴본 violation을

Eclipse를 사용해서 시정 조치하는 콤보를 살펴봅시다.

특정 소스 디렉토리에 대한 일괄 치환 방법

1. Eclipse 소스 디렉토리에서 원하는 디렉토리 선택
2. Eclipse 메뉴에서 'Search'  선택
3. 'File...' 선택
4. 'Containing text:' 입력 란에  탐색할 패턴 입력 (예: private static String ([A-Z]{1,}+))
5. 'Regular expression'  체크 박스 활성화
6. 'File name patterns (separated by comman):' 입력 란에 탐색할 파일 패턴 입력 (예: *.java)
7. 'Scope'에서 'Selected resources' 선택
8. 'Replace...' 버튼 선택
9. 'Replace Text Matches' 대화 상자의 'With:' 입력 란에 치환할 패턴 입력 (예:private static final String $1)
10. 'Preview >' 버튼 선택
11. 변경 전의 'Original Source', 변경 후의 'Refactored Source' 란에 입력된 소스 코드 비교
12. 정규 표현식 패턴에 잘 못 걸려 들어온 파일은 체크 박스 해제
13. 치환 대상이 이상 없음을 확인
14. 'OK' 버튼 선택
15. 일괄 치환 완료
16. 소스 코드 commit

자, 대략 이렇습니다. 이게 기본 동작입니다.

마음 같아서는 이미지 캡쳐 떠서 넣고 싶습니다만

재미도 감동도 없는 이미지 캡쳐 따위, 트래픽 낭비이니 생략하겠습니다.

실은 popit.kr 운영비 때문에 허덕인다고... 도움!

특별히 미리 보기로 변경 전, 후를 비교하지 않아도 된다면

'Preview > 부분은 생략하셔도 됩니다.

이걸 어떻게 다 마우스로 제어해서 컨트롤하냐... 생각하실 수도 있습니다만

스트리트파이터 게임에서 파동권을 쏠 때에도

조이스틱을 하, 우하, 우로 돌린 후, 주먹... 이렇게 익히지만

실전에서는 조이스틱을 마구 비벼 돌리면서 주먹을 연타합니다.

위 과정이 길어 보여도 몇 번 반복해보면 그렇게 하실 수 있습니다.

Round 2, Fight!

자, 이제 정규 표현식이 대충 무엇인지도 어렴풋이 알았고

코드 리뷰 결과에 대해 시정 조치를 할 때,

개발 툴의 기능으로 효과적으로 수정하는 방법도 알았습니다.

이제, 이 방법을 뇌로 기억하지말고

근육이 기억하도록 응용 한 번 해보겠습니다.

같은 동작을 일곱 번 반복하면

근육이 기억한다는 '머슬 메모리'가 활성화되도록

정규 표현식을 사용해서 소스 코드를 수정하는 동작을 해보도록 하겠습니다.

(머슬 메모리에 대한 의학적 검증은 아몰랑)

유형 2: FinalFieldCouldBeStatic

유형 2: 시술 전

1
private final String LIST_VIEW_NAME = "/xxxx";

움? 앞에서 본 유형과 왠지 비슷해 보이는데 뭐가 다른 걸까요?

눈치 빠른 사람은 이것이 Spring MVC의 Controller에서 view 이름을 선언한 것이라는 걸 알 수 있겠지만

지금 다루려는 주제와는 아무 상관 없습니다.

이런 코드에 PMD라는 정적 분석 툴을 걸어보면

FinalFieldCouldBeStatic이라는 이름의 위반으로 분류합니다.

내용인 즉,

'네가 이걸 final이라고 한 걸 보니, 추가로 static으로 처리해도 되지 않냐?'라는 것이고

곰곰히 생각해보니 사실 얼마나 더 좋아질지는 잘 모르겠지만

밑져야 본전, 손해볼 장사는 아닐 것이라고 팔랑귀가 팔랑팔랑거립니다.

그래서 이렇게 고쳐보려 합니다.

유형 2: 시술 후

1
private static final String LIST_VIEW_NAME = "/xxxx";

자, 뭐가 달려졌을까요? 라고 물어보기도 궁색하게 그냥 static 하나 더 들어갔습니다.

그러면 이런 패턴의 소스 코드가 많이 있으면 일괄 치환을 어떻게 해야할까요?

앞서 보셨던 유형 1과 비슷하니 패턴을 조금만 고쳐 보겠습니다.

유형 2: 정규 표현식으로 탐색할 패턴 (Find)

1
private final String ([A-Z]{1,}+)

유형 2: 정규 표현식으로 치환할 패턴 (Replace)

1
private static final String $1

네, 치환할 패턴은 앞서 본 유형 1과 같습니다.

Eclipse에서 치환하는 방법은 앞서 설명하였으니 다시 언급하진 않겠습니다.

다만, 같은 방식으로 직접 마우스를 제어하면서 툴을 익히고

정규 표현식을 직접 타이핑 해보면서

어디가 어떻게 치환되는지를

한 동작, 한 동작, 근육에 새겨 보시길 바랍니다.

오늘 우리가 찾은 것은 무엇인가, 잃은 것은 무엇인가, 남은 것은 무엇인가

이 제목의 출처가 무엇인지 알면 당신은 이미 아재

뭔가 장황하게 많은 내용을 다루었습니다만

오늘 우리가 배운 것은 무엇일까요?

한번 정리하고 가겠습니다.

우리가 찾은 것은 이것이다.

오늘은 정말 어처구니 없을 정도로 간단한 소스 코드를 가지고

그것도 동작하는데 별 지장 없어 보이는 악의 없는 코드를 가지고

온갖 호들갑을 떨면서 시정 조치를 하는 과정을 살펴보았습니다.

그러는 과정에서 정적 분석 툴 중의 하나인

PMD가 뱉어 내는 위반 사항 두 가지를 다루었습니다.

• FinalFieldCouldBeStatic
• VariableNamingConventions