【はじめての輪読会】リーダブルコード8章9章

サイオステクノロジーの佐々木です。 第2回輪読会を先日実施しました。 今回私の担当箇所は「巨大な式を分割する」「変数を削除する」という内容です。 変数を追加したほうがコードの可読性が上がる場合と逆に可読性が下がる場合の比較をしながら見ていただけると面白いと思います。

巨大な式を分割する

説明変数

if line.split(':')[0].strip() == "root":

if 文の左辺にある関数がパッと見何を表しているかわかりにくいですよね。 説明変数を使用して以下のように書き換えると左辺が短くなり、何を比較しているのかよりわかりやすくなります。

username = line.split(':')[0].strip()
if username == "root"

これで何を比較しているか分かりやすくなりましたね！

要約変数

if(request.user.id == document.owner_id){
	// 編集可能
}
if (request.user.id != document.owner_id){
	// 読み取り専用
}

上記の式はユーザがドキュメントの編集権限があるかないかなのでそれを表した変数に置き直すとよりわかりやすい

boolean user_owns_document = (request.user.id != document.owner_id)
if(user_owns_document) {
	// 編集可能
}
if (!user_owns_document) {
	// 読み取り専用
}

短絡評価の悪用

const galarCheck = sunnygo => {
  return sunnygo.type === "ゴースト" || sunnygo.type === ""
    ? (sunnygo.isGalar = true)
    : (sunnygo.type = "みず・いわ");
};

短絡評価は一行でかけてプログラマにとっては頭が良くなった気がしますが、半年後自分が読みやすいかもう一度考えてみると素直にif文で書いた方が分かりやすいでしょう

const galarCheck = sunnygo => {
  if (sunnygo.type === "ゴースト") {
    sunnygo.isGalar = true;
  } else if (sunnygo.type === "") {
    sunnygo.isGalar = true;
  } else {
    sunnygo.type = "みず・いわ";
  }
  return sunnygo;
};

複雑なロジックは優雅な方法を見つける

条件式が複雑になってきた場合どうするのがいいでしょうか？ 例えば二つの数列の範囲が被っているかどうかを確認するとしましょう。 その場合は以下のパターンが考えられます。

・最小値が入力された最小値よりも小さいかつ最大値が入力された最小値よりも小さい
・最大値が入力された最小値よりも大きいかつ最大値が入力された最大値よりも小さい
・最小値が入力された最小値よりも小さいかつ最大地が入力された最大値よりも大きい

日本語で書くとわかりにくいですね このように複雑化した条件は反対を考えると分かりやすくなることがあります。

・最小値が入力された最大値よりも大きい場合
・最大値が入力された最小値よりも小さい場合

このように考えると考慮する条件も減り比較する変数も一つになるので非常にわかりやすくなりますね。

巨大なコードを分割する

巨大なプログラムが出来上がってしまったら共通化部分を見つけ関数に切り出したり、説明変数、要約変数を駆使して分割して考えていきましょう。

変数と読みやすさ

先ほど可読性の低いものは説明変数や要約変数に置き換えてわかりやすくしましょうという話がありました。 しかし適当に変数に置き換えるとコードにゴミが残りプログラムの可読性を下げてしまいます。 以下に可読性を下げる変数の置き方の例をいくつか挙げます。

now = datetime.datatime.now()
const created_at = now

これは全く役に立たない変数の例でコードの重複を直す作業もしていなければdatetime.now()で十分意味を理解できるので変数が不要となる良い例です。

index = None
for i in range(len(array)):
	if array[i] == "ok":
		index = i
		break
print(array[index])

これは中間結果を保持するためだけにindexを使用していますが、結果をそのまま使用するようにした方がより簡潔になります

for i in range(len(array)):
	if array[i] == "ok":
		print(array[index])
		break

変数のスコープを縮める

変数のスコープを縮めることでプログラムを読んでいて考慮しないといけない変数を減らすことができます。 逆にJavaScriptでグローバル変数のようなスコープが広い変数を定義してしまうと意図しないときに変数が書き換えられてしまい不具合の原因となってしまう。

変数の書き込みは1度だけ

変数への書き込みが自由に行われると値の追跡をする難易度が格段に上がってしまい、現在の値の判断が難しくなってしまう。 プログラムで一度しか書き込むことができないという制約をかけることができるならば極力それを使うと、コードを追いやすくなる。 Javaでいうfinal、JavaScriptでいうconstを使っていきましょう。

議論したことのログ

1. 要約変数
   • is_Authとかでログインしていない場合のみの処理を行いたいとき、否定形で定義するかどうかを迷うというところからの議論アリ。booleanを使う場合は基本的には肯定系をtrueに定義するほうが良い。
2. 変数を削除する
   • 中間変数を削除してif文などの中で返すと、中間変数を削除できる。全くその通りだが、書き方の癖とかで忘れてしまいそう。
   • リーダブルコードの例が良くない気がしている。処理のタイミングで変わってしまうような情報（日時）は変数に格納しないとバグになるケースもある。一方で処理時点の日時が必要になる機能もあるので実装機能による話で、ちょっと例が微妙
   • リファクタリングカタログ「Replace Control Flag with Break」
3. 変数のスコープを縮める
   • グローバル変数のハードルが逆に高すぎてほとんど使わない
   • useEffectは第二引数をちゃんと書いてスコープを小さくする必要あり[count]
4. 変数は一度だけ書き込む
   • JSのオブジェクトはconstで定義しても更新できてしまうから、更新したくない値を定義する場合は注意。

終わりに

一人で呼んでいたらさらっと流してしまうような観点を複数人で読むと新しい観点が入り議論が深まってとても良い取り組みだと感じています。
今日早速本章で学んだことを実践して共有してくれるメンバーもいて、この実行力はすごいなと思い私も見習わないとと思います（笑）

About 佐々木寛太 21 Articles

普段はAPI開発をメインに最近はブロックチェーン周りに手を出し始めました。インフラが苦手なので今年はインフラ回りも攻めていこうかと思っています。休日は愛車とドライブしています。

Twitter YouTube