코드 리뷰를 잘하기 위한 Github 모범답안 모으기

이 글을 쓰게 된 계기

아카데미에서 여러 프로젝트를 하면서 많이 성장했다고 생각한다. 깃, 깃헙, Xcode, Swift,... 기타 등등 여러 부분에서 익숙해졌다고 생각했는데 짧은 기간 안에 습득할 수 없었던 역량이 하나 있었다. 그것은 바로 코드리뷰.. 그 어느 것 보다도 경험 있는 사람과 초보자의 격차가 많이 느껴지는 부분이 아닌가 생각한다. 그리고 그 격차에 의해 부담감과 미안함을 느끼기도 한다.

 

나는 기껏해야 오타를 봐줄 수 있는데, 잘하는 분들은 사소한 것 부터 아키텍처 / 더 좋은 코드를 쓰는 방법 등등 여러 팁 + 리뷰를 남겨주신다. 나도 얼른 잘 해서 동등한 퀄리티의 코드리뷰를 해 드리고 싶은 마음에 어떤 방법이 좋을까.. 생각을 해 보니 가장 좋은 개발 교과서인 깃헙을 활용하기로 했다.

 

현재 아카데미에서도 상당히 경력도 많고 뛰어난 분들이 많으신데, 깃헙은 고맙게도 그 분들이 작성하신 코드리뷰를 모두 담고 있기에 깃헙을 교과서 삼아 모범 코드리뷰 예시들을 모아 보기로 했다.

 

 

 

코드리뷰의 유형 정리

얼마 전 뛰어난 실력을 가진 개발자들과 팀을 이뤄 해커톤에 나간 적이 있었다.

그 때 거의 브리핑 하듯이 협업 놈즈를 알려주셨는데, 그때 있었던 코드리뷰 유형과 뱅크샐러드 블로그 글을 참고해서 정리해 보았다.

 

https://blog.banksalad.com/tech/banksalad-code-review-culture/

 

코드 리뷰 in 뱅크샐러드 개발 문화 | 뱅크샐러드

 

1. 작성자의 의도를 확인하는 질문
2. 버그가 발생할 수 있는 가능성 제시
3. 오타나 잘못된 네이밍 지적
4. 잘못 작성된 코드 정정 (어떤 함수나 클래스 등등을 잘못 이해하고 사용)
5. 다른 해결 방법에 대한 의견 제시
6. 라이브러리 등 서드파티 제안
7. 기술적인 지식, 노하우 공유
8. 일관된 아키텍처를 유지하고 있는지

 

등등의 코드리뷰 유형이 있을 것이다.

 

그리고 코드리뷰를 할 때 다섯 단계를 두는 것을 실천했었는데, 뱅크샐러드에서 실제로 이런 방식을 사용하고 있다고 한다.

 

 

 

실제 Github 보면서 유형 탐색해보기

네이밍 변경 제안

• commonInit() 보다는 setupViews() 라는 보다 명확한 이름을 쓰는 게 어떨까요?
• UIKit에서는 뒤에 View를 붙여주는 것이 일반적인 명명이랍니다~!

 

더 좋은 코드 제안

• private method를 별도의 extension 으로 빼는 것은 어떨까요?
• students 배열을 외부에서 주입받도록 구현하는게 어떨까요? 어차피 나중에 의존성을 외부에서 주입받게 구현해야할 것 같고, 프리뷰를 볼 때도 여러 경우를 보기 위해서 initializer 를 수정하는게 좋아보입니다!
• if문 대신 guard 문을쓴다면 인덴트도 사라지고, 빠르게 리턴할 수 있지 않을까합니다~!
• private var isInitialzed: Bool = false 에서 Bool은 생략이 가능합니다
• 외부에서 해당 함수를 건들이지 않는 거 같아서 private으로 설정해주는 건 어떨까요?
• 내부에서 리턴해줄 값이 없다면 private let searchBarView = SearchBarView() 이런식은 어떨까요

// 기존 코드
private let searchBarView: SearchBarView = { 
    let view = SearchBarView() 
    return view 
}()

 

잘못 작성된 코드 정정

• override 메소드에서는 부모 class의 실행이 되도록 super 함수 실행이 중요해요!
• 이 부분은 비동기적인 코드라서 await 를 함께 작성해야 맞을 것 같습니다

 

작성자의 의도 확인 (보통 답변도 상세하게)

• private extension 이라서 메소드들 private 으로 선언해주지 않아도 되는건가요??

→ 넵! private extension 이면 안에있는 메소드들은 전부 private 으로 접근 제어가 된다고 합니다

• 상수형관련 코드들인데 컨트롤러 안에 넣어둔 이유가 있을까요?

→ 해당 Size 값들을 다른 Class들은 사용하지 않는데, 따로 파일을 만들어서 internal하게 사용하는 거 자체가 아닌 거 같아서 VC class 내부에서 private하게 사용하도록 구현했습니다.

 

중복된 코드 지적

• window 를 만들 때 scene 과 함께 만들었는데, windowScene 에 다시 scene 을 넣을 필요가 있을까요?

window = UIWindow(windowScene: scene) 
window?.rootViewController = UISplitViewController() window?.windowScene = scene

• import SwiftUI에 포함이 되어 있기 때문에 import Foundation은 삭제해도 될 것 같습니다!

 

오타나 사소한 부분 정정

• 안 쓰는 코드라면 삭제해도 좋을 것 같습니다! (주석 등등)

 

버그가 우려되는 부분

• !보다는 optional을 해제하거나 하는 안전한 방법으로 하면 어떨까요 ?

 

일관성 유지

• 이 부분도 다른 파일의 코드들과 일관성을 유지할 수 있도록 setUpDelegate 함수 안에서 작성해주시면 보기 더 좋을 것 같아요

 

컨벤션 지킴이

• import 컨벤션 알파벳 순인데 확인 부탁드립니다!

 

 

결론 (but ing…)

생각보다 코드리뷰 내용이 많이 겹친다. 그리고 단순히 코드리뷰 유형만 익혀서는 안되고, Swift와 아키텍처 그리고 좋은 코드에 대한 이해가 필수적으로 필요하다 (❗️매우 중요❗️)

 

근데! 당장 그걸 익히기는 힘드니 앞으로 계속 계속 추가를 해 나가려고 한다. 좋은 코드리뷰를 볼 때마다..

그리고 또 중요한 것은 내 PR 에 달린 코드 리뷰가 아니더라도 계속 살펴보면서 이해하려고 하는 노력이 필요할 것 같다. 그렇지 않으면 좋은 코드리뷰를 그냥 지나치는 셈이니까!