Dialy

リーダブルコードを読んだら凄い本だったので簡単な実践をしてみた

更新日:2022-12-04 公開日:2022-01-16

今日はプログラマーの必読書として名高いリーダブルコードを読み終えたので、その感想を書いていきます。

書籍概要

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

(原題:The Art of Readable Code: Simple and Practical Techniques for Writing Better Code)
Dustin Boswell (著)Trevor Foucher (著)
須藤 功平 (解説), 角 征典 (翻訳)

「美しいコードを見ると感動する。優れたコードは見た瞬間に何をしているかが伝わってくる。そういうコードは使うのが楽しいし、
自分のコードもそうあるべきだと思わせてくれる。本書の目的は、君のコードを良くすることだ」(本書「はじめに」より)。

コードは理解しやすくなければならない。本書はこの原則を日々のコーディングの様々な場面に当てはめる方法を紹介します。
名前の付け方、コメントの書き方など表面上の改善について。コードを動かすための制御フロー、論理式、変数などループとロジックについて。
またコードを再構成するための方法。さらにテストの書き方などについて、楽しいイラストと共に説明しています。

日本語版ではRubyやgroongaのコミッタとしても著名な須藤功平氏による解説を収録。

※Amazon.co.jpのサイトより引用




全体を通しての感想

結論から先に言うと、ためになる上に滅茶苦茶おもしろいです。

主にJavaとC++とPythonとJavaScriptがサンプルとして使われていますが、
全般的に長いコードは掲載されていないので、カバーしていない言語でも頭の中に入ってくるでしょう。

特定の言語を題材にしているわけではなく、各言語に共通して言えることが書かれているためです。
例えば、変数名や関数名のつけ方だったり、効果的なコメントの方法、ループ処理の制御方法などです。

内容の紹介

いくつかサンプルコードと解説を引用します。

2章 名前に情報を詰め込む


「名前に情報を詰め込む」には、明確な単語を選ばなければいけない。「空虚な」単語は避けるべきだ。
 例えば、「get」はあまり明確な単語ではない。

 def GetPage(url):
  ...

「get」という単語からは何も伝わってこない。このメソッドはページをどこから取ってくるのだろう?
ローカルキャッシュから?データベースから?インターネットから?
インターネットから取ってくるのであれば、FetchPage()やDownloadPage()の方が明確だ。

2.1 明確な単語を選ぶ より


こんな感じで各言語の共通的な話題が多いです。

5章 コメントすべきことを知る


プログラマの多くはコメントを書きたがらない。コメントをうまく書くのは大変だと思っているからだ。
こうした「ライターズブロック」を乗り越えるには、とにかく書き始めるしかない。
自分の考えていることをとりあえず書き出してみよう。生煮えであっても構わない。
 例えば、ある関数を作っていて、「ヤバい、これはリストに重複があったら面倒なことになる」と思ったとする。
それをそのまま書き出せばいい。

// ヤバい。これはリストに重複があったら面倒なことになる。

ね、簡単でしょう?変なコメントでもないよね(何もないよりはマシだ)。


ところどころにユーモアを交えて解説しているのも特徴です。
しかし、このコメントの発想はなかったという感じです。確かに何もないよりマシでしょう。

もちろんこれだけで終わらず、その後も続きます。

ただ、ちょっとあいまいな表現かもしれない。言い回しをもっと詳細な言葉に置き換えるといいだろう。
・「ヤバい」は「注意:これには気を付けて」という意味だ。
・「これ」は「入力を処理するコード」という意味だ。
・「面倒なことになる」は「実装が難しくなる」という意味だ。
書き換えたコメントは以下のようになる。

// 注意:このコードはリストの重複を処理できません(実装が難しいので)。

5.4 ライターズブロックを乗り越えるより


こんな具合で非ロジック的なためになる解説が多いので、すらすらと読めるのです。
ちなみにコメント技法については5章だけでなく、6章も解説が続きます。
コメントの仕方についてこんなに書いている本が他にあるでしょうか、私は知りません。

7章 制御フローを読みやすくする

もちろん名前の付け方やコメント技法だけでなく、ロジックについての解説もあります。
例えばこの章では以下のような内容が記載されています。

  • 条件式の引数の並び順
  • if/elseブロックの並び順
  • 三項演算子
  • do/whileループは避ける
  • 関数から早く返す


どれも各言語に共通して言えそうですね。
特にdo/whileループは良く分からんと常々思っていたのですが、この本が良くない点を言語化してくれています。

do/whileループが変わっているのは、コードブロックを再実行する条件が下にあることだ。
if文・while文・for文などの条件は、コードブロックの上にある。
コードは上から下に読んでいくので、do/while文は少し不自然だ。
コードを2回読むことになってしまう。

7.4 do/whileループを避ける より


せっかくなので実践をしてみた

自分のコードを本に倣って改善していきたいと思います。
題材は、ちょっとした社内業務にPython Djangoの予約システムを稼働させているのですが、その中の重複チェックのスクリプトです。
部屋を予約しようとする際に、既に予約済の時間があった場合、予約不可とする、みたいなイメージです。

コードはブログ用に改編していますが、ロジックと流れは実業務で使っているものとほぼ同じです。

元コード


import datetime


def check_duplicate(booking_date: str, start_time: str, end_time: str):
    """予約時間が既に取られていないか確認して結果を出力"""

    # 予約リスト 実際はデータベースから値を取得
    already_booked_list = [{'date': '2022-01-15', 'start_time': '9:00', 'end_time': '10:00'},
                           {'date': '2022-01-16', 'start_time': '9:00', 'end_time': '10:00'},
                           {'date': '2022-01-16', 'start_time': '10:00', 'end_time': '11:00'},
                           {'date': '2022-01-16', 'start_time': '12:00', 'end_time': '13:00'},
                           {'date': '2022-01-16', 'start_time': '15:00', 'end_time': '16:00'},
                           {'date': '2022-01-16', 'start_time': '16:00', 'end_time': '17:00'}]

    # 重なっていたらTrueにする
    flag = False
    for taken in already_booked_list:
        # 予約日が一緒だったら重複をチェックする
        if booking_date == taken['date']:
            # 比較するためにdatetime型に変換していく

            # これから予約する時間
            start_time_for_book = booking_date + ' ' + start_time
            start_time_for_book = datetime.datetime.strptime(
                start_time_for_book, "%Y-%m-%d %H:%M")

            end_time_for_book = booking_date + ' ' + end_time
            end_time_for_book = datetime.datetime.strptime(
                end_time_for_book, "%Y-%m-%d %H:%M")

            # 予約済みの開始時間
            start_time_taken = taken['date'] + ' ' + taken['start_time']
            start_time_taken = datetime.datetime.strptime(
                start_time_taken, "%Y-%m-%d %H:%M")

            # 予約済みの終了時間
            end_time_taken = taken['date'] + ' ' + taken['end_time']
            end_time_taken = datetime.datetime.strptime(
                end_time_taken, "%Y-%m-%d %H:%M")

            # 重複している場合flagをTrueにする

            # 開始時間が同じ
            if start_time_taken == start_time:
                flag = True
            # 終了時間が同じ
            if end_time_taken == end_time:
                flag = True
            # 開始時間が予約済み内
            if start_time_taken < start_time_for_book < end_time_taken:
                flag = True
            # 終了時間が予約済み内
            if start_time_taken < end_time_for_book < end_time_taken:
                flag = True
      # オーバーライドしている
      if start_time_for_book < start_time_taken and end_time_for_book > end_time_taken:
          flag = True

    if flag:
        print(f'予約可能! {booking_date} {start_time}-{end_time}')
    else:
        print('重複していない')


check_duplicate関数はこれから予約する日時を受け取り、予約済みリストと重複をチェックします。
改めて見るとひどいコードですね。

さっそく本に倣って整理していきましょう。

無関係の下位問題を抽出する

リーダブルコードでは、無関係の下位問題を抽出するという技法が紹介されています。
ある関数でやりたいことを明確にした上で、無関係の下位問題を抽出して別関数でまとめる、ということです。
check_duplicate関数でやりたいことは時間の重複チェックですね。

なので、この辺りが下位問題にあたる部分でしょう。

start_time_for_book = booking_date + ' ' + start_time
start_time_for_book = datetime.datetime.strptime(
    start_time_for_book, "%Y-%m-%d %H:%M")


文字列の日付を比較するためにdatetime型に変換するくだりです。
しかもこれは計4回登場します。正直うざったいですね。

ここを関数で切り出します。

def convert_datetime(date: str, time: str):
    """日付文字列と時間文字列をdatetime型にして返す"""
    date_string = f'{date} {time}'
    return datetime.datetime.strptime(date_string, "%Y-%m-%d %H:%M")


あとは呼び出します。

for taken in already_booked_list:
    # 予約日が一緒だったら重複をチェックする
    if booking_date == taken['date']:
        # 比較するためにdatetime型に変換していく

        # これから予約する時間
        start_time_for_book = convert_datetime(booking_date, start_time)
        end_time_for_book = convert_datetime(booking_date, end_time)
        # 予約済みの開始時間
        start_time_taken = convert_datetime(taken['date'], taken['start_time'])
        # 予約済みの終了時間
        end_time_taken = convert_datetime(taken['date'], taken['end_time'])


大分すっきりしました。

ネストを浅くする

if 文の中にif文があると読みづらいです。
ネストが深くなるにつれ、読んでいる際に常に自分がどの条件にいるのか、ということを考えないといけないですね。
なので、ネストを浅くしてみましょう。

for taken in already_booked_list:
    # 変更 日付が違う場合はチェックの必要がない
    if booking_date != taken['date']:
        continue

    # 比較するためにdatetime型に変換していく

    # これから予約する時間
    start_time_for_book = convert_datetime(booking_date, start_time)
    end_time_for_book = convert_datetime(booking_date, end_time)
  ...


これで一段ネストが浅くなりました。

ライブラリを知る

でも待ってください。そもそも日付の一致をチェックする必要があるのでしょうか。
リーダブルコードでは簡潔なコードを書くのに欠かせないのは、ライブラリが何を提供してくれるか知ることであるとしています。
例えばこのコードについても日付を順番にぶつけていますが、リストが膨大になった時にパフォーマンスにも影響が出そうです。
Pythonではlamda関数を使うことで、辞書のリストもフィルタリングができます。
以下のようにすると、これから予約する日付で絞り込むことができます。

# リストをあらかじめ予約日付で絞り込む
already_booked_list = list(filter(lambda x: x['date'] == booking_date, already_booked_list))
for taken in already_booked_list:
    # 日付確認がいらなくなった
    # 比較するためにdatetime型に変換していく

    # これから予約する時間
    start_time_for_book = convert_datetime(booking_date, start_time)
    end_time_for_book = convert_datetime(booking_date, end_time)
...


これでfor文の中の時間重複ロジックだけに集中できるようになりました。

より優雅な手法を見つける

さて、いよいよ時間の重複ロジックの見直しに入っていきます。
ここの部分ですね。

# 開始時間が同じ
if start_time_taken == start_time:
    flag = True
# 終了時間が同じ
if end_time_taken == end_time:
    flag = True
# 開始時間が予約済み内
if start_time_taken < start_time_for_book < end_time_taken:
    flag = True
# 終了時間が予約済み内
if start_time_taken < end_time_for_book < end_time_taken:
    flag = True
# オーバーライドしている
if start_time_for_book < start_time_taken and end_time_for_book > end_time_taken:
    flag = True


複雑なロジックです。
リーダブルコード内で似たような題材がありました。
コードはJavaですが、OverlapsWithという範囲の重なりをチェックする関数がサンプルとして紹介されています。
このような問題は場合分けや条件が多すぎて、バグを見逃しやすいとのことです。

実際に自分もバグを見逃していて今日気が付きました。
( // ヤバい 仕事で使ってるから早く直さないと )

例えば12:00から13:00の時間は予約されているのに、以下のコードは予約可能と出力されてしまいます。

check_duplicate(booking_date='2022-01-16', start_time='11:00', end_time='13:00')


どのようなif文を追加すればいいのでしょうか、とはいえ考えたくもない問題です。

さて、こういう場合は立ち止まって、全く違った手法を考えてみるいいと記述されています。


「反対」から問題を解決してみるという手がある。例えば配列を逆順にイテレートしてみる。データを後ろから挿入してみる。
とにかくいつもと反対のことをやってみるのだ。
OverlapsWith()の「反対」を考えると「重ならない」になる。2つの範囲が重ならないのは簡単だ。
2つの場合しかない。
1.一方の範囲の終点が、ある範囲の始点よりも前にある場合。
2.一方の範囲の始点が、ある範囲の終点よりも後にある場合。
これをコードに置き換えるのは簡単だ。

8.5 例:複雑なロジックと格闘する より


そういわれてみるとそうなんですけど、気が付かなかった部分です。
私は「重なっている条件」のみをずっと考えていたのが間違っていたわけです。

早速今回のケースに当てはめて実装してみました。
全文を掲載します。

def convert_datetime(date: str, time: str):
    """日付文字列と時間文字列をdatetime型にして返す"""
    date_string = f'{date} {time}'
    return datetime.datetime.strptime(date_string, "%Y-%m-%d %H:%M")


def check_duplicate(booking_date: str, start_time: str, end_time: str):
    """予約時間が既に取られていないか確認して結果を出力"""

    # 予約リスト 実際はデータベースから取得
    already_booked_list = [{'date': '2022-01-15', 'start_time': '9:00', 'end_time': '10:00'},
                           {'date': '2022-01-16', 'start_time': '9:00', 'end_time': '10:00'},
                           {'date': '2022-01-16', 'start_time': '10:00', 'end_time': '11:00'},
                           {'date': '2022-01-16', 'start_time': '12:00', 'end_time': '13:00'},
                           {'date': '2022-01-16', 'start_time': '15:00', 'end_time': '16:00'},
                           {'date': '2022-01-16', 'start_time': '16:00', 'end_time': '17:00'}]

    # 重なっていたらTrueにする
    flag = False
    # リストをあらかじめ予約日付で絞り込む
    already_booked_list = list(filter(lambda x: x['date'] == booking_date, already_booked_list))
    for taken in already_booked_list:

        # これから予約する時間
        start_time_for_book = convert_datetime(booking_date, start_time)
        end_time_for_book = convert_datetime(booking_date, end_time)

        # 予約済みの時間
        start_time_taken = convert_datetime(taken['date'], taken['start_time'])
        end_time_taken = convert_datetime(taken['date'], taken['end_time'])

        # 1.一方の範囲の終点が、ある範囲の始点よりも前にある場合。
        if end_time_for_book <= start_time_taken:
            continue
        # 2.一方の範囲の始点が、ある範囲の終点よりも後にある場合。
        elif start_time_for_book >= end_time_taken:
            continue
        # 残ったものは重なっている
        else:
            flag = True

    # 結果出力
    if flag:
        print(f'重複している! {booking_date} {start_time}-{end_time}')
    else:
        print(f'予約可能! {booking_date} {start_time}-{end_time}')


どうでしょう、変数名のつけ方などまだまだ改善の余地がありそうですが、初期に比べると大分すっきりしたと思います。

おわりに

リーダブルコードは冒頭でも書いた通り、特定の言語のことが分からなくても読めるように工夫がされています。
極論を言えば、プログラミングをやったことがない人が読んでも楽しめるのではないかと思いました。
何より、コーディングに対して前向きになれるというか、意欲が生まれる素晴らしい本です。