Python の言語仕様を理解していなかったため、危険なバグを発生させた話

最初、このような関数がありました

<br>def is_in_payment_term(item) -> bool:
"""
引数に取ったアイテムが購入可能かをチェック
"""
    now = datetime.now()

    # 本当はもっと色々条件があるのですが…
    return  item.payment.term_start <= now <= item.payment.term_end

この書き方だとテストを書くのが大変でした。 現在時刻を元にn日後、などと書けば書けなくはないのですが 例えば日や月や年をまたぐ場合など、バグが発生しそうな境界値でチェックを行う処理をテストすることは とても複雑な条件を書いたり、datetime で返す時間を変更したりする必要がありました。

関数をこのように書き換えました

関数型プログラミングって何？ という文章をちょっと前に読んでいた私は 「移せる物は引数に移せばいい」と考えました そこで、先ほどの関数を以下のように書き換えました

<br>def is_in_payment_term(item, now=datetime.now) -> bool:
"""
引数に取ったアイテムが購入可能かをチェック
"""
    return  item.payment.term_start <= now <= item.payment.term_end

これでかなりテストも書きやすくなりました。 ただ引数を増やしただけでは、元々動いていたコードが動かなくなってしまうので、 now はデフォルト値で現在時刻を取るようにしました。 これで、テスト時は now に引数を与えれば任意の現在時刻で実行でき、元のコードは何も書き換える必要がありません。

バグの発覚

その後、私はいろんなところでこの書き方をしていました 例えば、以下のような場所でも

<br>def add_trail(db, user_id, page, operation, now=datetime.now()):
"""
ユーザの操作ログをDBに記録する
"""
    db.session.add( ... )  # 割愛

この関数で作られたログを見ていると、おかしなことに気がつきました

+----+---------+---------------------+--------------+-------------------------+
| id | user_id | mod_datetime        | page_type    | operation_type          |
+----+---------+---------------------+--------------+-------------------------+
| 22 |       1 | 2017-01-26 15:02:54 | プロジェクト | 順序を変更              |
| 23 |       1 | 2017-01-26 15:02:54 | プロジェクト | 順序を変更              |
| 24 |       1 | 2017-01-26 15:02:54 | プロジェクト | 順序を変更              |
| 25 |       1 | 2017-01-26 15:02:54 | ユーザー     | 退会                    |
| 26 |       1 | 2017-01-26 15:34:17 | ユーザー     | 退会                    |
| 27 |       1 | 2017-01-26 15:34:17 | プロジェクト | 公開状態を変更          |
| 28 |       1 | 2017-01-26 15:34:17 | お知らせ     | 追加                    |
| 29 |       1 | 2017-01-26 15:34:17 | お知らせ     | 更新                    |
| 30 |       1 | 2017-01-26 15:34:17 | お知らせ     | 削除                    |
| 31 |       1 | 2017-01-26 15:34:17 | 管理者       | 追加                    |

なんか、時間のパターンが少ない…？ ユーザを一人退会させてから、新たなユーザを追加するまで1秒も経過してません。

時間がおかしい原因

たしか操作ID 25 から 26 の間はバグに気がついて修正し、プログラムを再起動させました。 ということは、これってプログラムを起動させた時間のまま now の値が止まっていることになります。

def function(now=datetime.now):

このように書くと、now の値は関数呼び出しの時ではなくてプログラム実行時に決定されてしまうのです。 確かに、毎回計算してたら「デフォルト引数」で設定しているのにリソースの無駄ですよね。確かに、実行時に値を固定させますよね。

ということは、最初に書いた期間チェックの関数もマズいことになります。 (再掲)

<br>def is_in_payment_term(item, now=datetime.now) -> bool:
"""
引数に取ったアイテムが購入可能かをチェック
"""
    return  item.payment.term_start <= now <= item.payment.term_end

これも、購入可能かチェックするための基準時間が、プログラムを起動させた時間で固定されることになります。 つまり、 いつで経っても購入終了時刻がやって来ないことになります。 …気づいて良かった。

デフォルト引数に datetime.now を渡さないように修正

<br>def is_in_payment_term(item, now=None) -> bool:
"""
引数に取ったアイテムが購入可能かをチェック
"""
    if now is None: now = datetime.now
    return  item.payment.term_start <= now <= item.payment.term_end

これで、(あんまり綺麗ではないですが)引数になにもなければ現在時刻が与えられ、引数に現在時刻を渡すことも出来るようになりました。

おわりに

• Python のデフォルト値はプログラム起動時に決定されます
  • これって他の言語ならばどうなるんでしょうか？
• 今のところは↑のような書き方をしていますが、もっといい書き方をご存じの方がいれば教えて下さい