iTranslated by AI

The content below is an AI-generated translation. This is an experimental feature, and may contain errors. View original article
🧑‍🔬

[Notes] Avoid blindly using if-else for two-way branching

に公開

Introduction

When writing conditional branches between two parties, if~else is often used.
However, using if~else can decrease readability and extensibility in some cases.
In this article, I will introduce problems that arise from using if~else.
Note that the code examples here are written for Ruby on Rails.

For example, when comparing the ages of three people—Yamada, Nakajima, and Ito—and retrieving the oldest person, you can use the max_by method like this:

  yamada = User.create!(name: "Yamada", age: 20)
  nakajima = User.create!(name: "Nakajima", age: 40)
  ito = User.create!(name: "Ito", age: 30)
  [yamada, nakajima, ito].max_by{|user| user.age} # nakajima is returned

Please refer to the following link for the max_by method.

https://docs.ruby-lang.org/ja/latest/method/Enumerable/i/max_by.html

Now, what if we compare the ages of just two people—Yamada and Nakajima—and retrieve the name of the older person?
Following the logic above, you could write it like this:

  max_age_user = [yamada, nakajima].max_by{|user| user.age} # nakajima is returned
  max_age_user.name # Nakajima is returned

However, in practice, it is sometimes written like this:

  if yamada.age > nakajima.age
    max_age_user = yamada
  else
    max_age_user = nakajima
  end
  max_age_user.name # Nakajima is returned

You probably use if~else for conditional branching between two parties most of the time.
However, personally, I prefer to avoid using if~else as much as possible.

Problems with using if~else

Reason 1: Poor readability

Using conditional branches increases the number of lines,
which forces the reader to parse more logic and increases cognitive load.

Reason 2: Poor extensibility

When using conditional branches, you need to modify the code every time a new condition is added.
Using if~else can lead to complex code as a result of adding elsif clauses.

Let's look at actual cases I've encountered regarding these two reasons.

When conditions are added

When another developer modifies the code to add a condition,
they might simply modify the conditional branch itself.
Consider the following code using if~else:


if hoge.content.present? && fuga.content.present?
  "Hoge・Fuga"
elsif hoge.content.present? && !fuga.content.present?
  "Hoge"
elsif !hoge.content.present? && fuga.content.present?
  "Fuga"
end

In this process, if the content of each of the two objects, hoge and fuga, exists, it returns the corresponding string.
Readability is already low at this point, but when the number of objects increases, it might be written like this:


if hoge.content.present? && fuga.content.present? && piyo.content.present?
  "Hoge・Fuga・Piyo"
elsif hoge.content.present? && fuga.content.present?
  "Hoge・Fuga"
elsif hoge.content.present? && piyo.content.present?
  "Hoge・Piyo"
elsif fuga.content.present? && piyo.content.present?
  "Fuga・Piyo"
elsif hoge.content.present?
  "Hoge"
elsif fuga.content.present?
  "Fuga"
elsif piyo.content.present?
  "Piyo"
end

As the number of objects increases, the number of conditional branches grows.
In this case, the following approach should have been considered from the start:


tmp = []
tmp << "Hoge" if hoge.content.present?
tmp << "Fuga" if fuga.content.present?
tmp.join("・")

In Case of Nesting

Consider cases where conditional branches are nested as shown below.

  hoge = Hoge.find_by(id: params[:hoge_id])
  fuga = Fuga.find_by(id: params[:fuga_id])
  if hoge.present? && fuga.present?
    if hoge.created_at > fuga.created_at
      name = hoge.name
      date = hoge.created_at.strftime("%Y年%m月%d日")
    else
      name = fuga.name
      date = fuga.created_at.strftime("%Y年%m月%d日")
    end
  elsif hoge.present?
    name = hoge.name
    date = hoge.created_at.strftime("%Y年%m月%d日")
  elsif fuga.present?
    name = fuga.name
    date = fuga.created_at.strftime("%Y年%m月%d日")
  end

You can write this without nesting the conditional branches like this:

  hoge = Hoge.find_by(id: params[:hoge_id])
  fuga = Fuga.find_by(id: params[:fuga_id])

  content = [hoge, fuga].compact.max_by(&:created_at)
  return unless content

  name = content.name
  date = content.created_at.strftime("%Y年%m月%d日")

Summary

When comparing conditions between two parties, if~else is often used. However, considering readability and extensibility, I believe it is desirable to eliminate conditional branches at an early stage by utilizing arrays and other techniques. Since using arrays or objects can sometimes make the logic harder to follow—so I cannot say such modifications are always better—I recommend thinking of ways to eliminate conditional branches instead of using them thoughtlessly.

GitHubで編集を提案

Discussion