Keep if clauses side-effect free
if 조건문에는 부작용을 넣지 마세요
if 조건문은 조건을 검사하는 데 쓰고, 상태를 바꾸는 호출은 먼저 별도 문장으로 실행하자는 제안입니다. 동작과 검사 과정을 분리하면 반환값의 뜻이 분명해지고, 짧게 훑어볼 때도 중요한 호출을 놓치기 어렵습니다.
- 주제
AI 요약
if 조건식 안에서 상태를 바꾸는 함수를 호출하면 동작과 조건 검사가 한데 섞입니다. 글쓴이는 부작용이 있는 호출을 먼저 실행하고 결과를 변수에 담은 뒤, 그 값을 if에서 검사하자고 제안합니다. 몇 줄을 아끼는 것보다 코드의 동작을 쉽게 파악하는 편이 낫다는 주장입니다.
호출 결과에 이름 붙이기
if (enqueueMessage(message))는 반환값이 메시지를 큐에 넣었다는 뜻인지, 큐가 가득 찼다는 뜻인지 바로 알기 어렵습니다. 먼저 boolean success = enqueueMessage(message);로 실행 결과를 담으면 if (success)가 되어 조건의 의미가 분명해집니다.
같은 원칙은 정수 반환값에도 적용됩니다. if (flushQueue() == 0)보다 int itemsFlushed = flushQueue();를 먼저 쓰고 if (itemsFlushed == 0)으로 검사하면, 큐를 비우는 동작과 결과 확인이 나뉩니다. 코드를 훑어보는 독자가 함수 호출을 단순한 조회로 오해하거나 아예 놓칠 가능성도 줄어듭니다.
조건식 안의 상태 변경은 읽기 어렵습니다
글쓴이는 운영 코드에서 본 if (!categorySeen.add(categoryID)) continue;를 예로 듭니다. Set.add()는 원소가 새로 추가됐을 때 true를 반환하지만, 이 코드는 continue, 부정 연산자, API 반환 규칙에 걸친 부정이 겹쳐 동작을 파악하기 어렵습니다. boolean isNewCategory = categorySeen.add(categoryID);로 먼저 추가하고 if (isNewCategory)로 분기하면 의미가 드러납니다.
단락 평가에 부작용을 숨기지 않기
if (queueNeedsFlushing() && flushQueue() == 0)에서는 두 번째 호출이 눈에 잘 띄지 않습니다. 글쓴이는 &&의 단락 평가를 나눗셈 오류 방지나 null 검사처럼 부작용 없는 조건을 보호하는 데 쓰고, 상태 변경 호출을 생략하는 수단으로 삼지 말라고 합니다. queueNeedsFlushing()을 먼저 검사하고, 참일 때 flushQueue()를 실행한 뒤 결과를 확인하면 실행 순서가 분명해집니다.
Lobsters 반응
- @prayerie — 첫 번째
success예시는 불필요하게 장황하다고 봅니다. 함수 호출을 if에서 검사한다면 성공 여부와 관련 있으리라는 점은 대개 분명합니다. “if 문은 조건이 참인지 검사하는 역할만 한다”는 말은 누가 정한 규칙인가요?- @cpurdy — 이 블로그는 이상하며 대체로 무시하는 편이 낫다고 봅니다. 임시 변수를 만들어 컴파일러가 할 일을 직접 하고 싶다면 그렇게 해도 되지만, 자신과 함께 일하는 사람에게 강요하지는 말아야 합니다. 함수 이름과 계약을 먼저 살펴 호출의 뜻을 분명히 해야 합니다. 가독성은 중요하지만, 글의 예시는 가독성을 높인다면서 오히려 낮춥니다.
- @gavinmorrow — 그 예시가 어색한 이유는 부작용 자체보다 불리언을 쓰는 방식에 있다고 봅니다.
if (enqueueMessage().is_ok())라면 괜찮거나 조금 나을 수 있습니다. 다만 부작용이 있는 함수는 동작을 이름에 담는 경우가 많아 반환 상태까지 한 줄에 쓰면 어색해집니다. 변수에 담으면 불리언에 이름을 붙여 자연스럽게 읽힙니다.
- @wareya — 부작용 자체는 괜찮지만, 발생한다는 점을 코드에 드러내야 한다고 생각합니다.
if (handle_event(x) == EV_OK)는 괜찮지만if (user_callback(x) == EV_OK)는 그렇지 않습니다. 주석도 도움이 되지만 함수 이름을 고치는 편이 낫습니다. if 안의 부작용은 구조와 형식 면에서 유용하므로, 프로그래머가 생길 수 있음을 의식해야 합니다. 사람은 if를 순수한 논리식으로 읽는 경향이 있으니 코드 형태로도 알림을 줘야 합니다.- @Ambroisie —
user_callback은 코드 작성자가 제어하지 않는 함수이므로 부작용이 있을 수 있다고 봐야 합니다. 요점에는 동의하지만, 예시만큼은 다르게 생각합니다. 결국 코딩 스타일 문제로 돌아옵니다.
- @Ambroisie —
- @chrismorgan —
boolean isNewCategory = categorySeen.add(categoryID);에서 문제는add가 불리언을 반환하는 점이라고 봅니다. 반환값이 “추가됨” 또는 “이미 집합에 있음”이라면 명확합니다. Rust의HashSet::insert도 비슷하게 불리언을 반환합니다. 별도 열거형을 쓰면Insert::Inserted같은 비교에 추가 import가 필요해 사용성이 좋지 않을 수 있습니다.HashMap::insert는 교체한 값이 있으면Option<V>를 반환하므로 모호하지 않습니다.- @jessicah — .NET의
TryAdd계열 함수는 if 안에서도 자연스럽게 읽혀 좋습니다.true일 때 참조를 돌려주는out매개변수 형태도 유용합니다. nullable 주석을 함께 쓰면 IDE가 각 분기에서 null 안전 여부를 파악하고, 바인딩 패턴은 더 낫다고 생각합니다. Rust도 비슷한 기능이 있습니다.
- @jessicah — .NET의
- @veqq — 부작용을 let 블록에 넣는 방식도 좋아합니다. 예를 들어
test.db경로를 정하고, 파일이 있으면 지운 다음 데이터베이스를 열 수 있습니다.defer로 파일 삭제와 데이터베이스 종료를 예약한 뒤 SQL을 실행합니다. - @bediger4000 — Go의 짧은 if 문은 어떨까요?
if entry, ok := dosomething(key); ok { ... }처럼 씁니다. 이 문법은entry와ok의 범위를 본문으로 제한해 인지 부담을 줄이는 데 목적이 있다고 봅니다. - @joshka — 한 줄에는 대체로 실패할 경로를 하나만 두자는 규칙도 좋아합니다. 구조화된 오류 처리에서는 조건문이나 반복문과 오류 발생을 한데 섞거나, 한 호출 연쇄에서 여러 예외가 생기게 하지 않는 편이 좋습니다. 이런 방식은 실행 추적과 로그, 장애 뒤 진단에 도움이 됩니다. 여러 언어와 시스템에서 이런 문제를 겪은 경험이 있습니다.
- @conor — 글의 주장에 동의합니다. 같은 이유로 Walrus Operator도 싫어합니다. 성능 이득 없이 한두 줄을 줄이는 대신 코드를 읽기 어렵게 만듭니다.
- @dpedu — 2003년 Linux 백도어 시도가 떠오릅니다. 부작용이 문제였죠.
- @nytpu — C에서는 이 스타일을 두고 오래 고민했지만, Rust와 Lisp에서
while let이나if let같은 패턴을 흔히 쓰는 걸 접한 뒤에는 가독성을 크게 해치지 않는다고 봅니다. 특히 C의 오류 처리 관례에서는 반환형이 다른 임시 변수를 여러 개 선언하는 일도 번거롭습니다. 뜻이 정말 불분명한 경우가 아니라면 함수를 조건식 안에 두는 편이 낫습니다.
원문: Team Ten / 번역·요약: Trawling