כמה סיבות לשפר קוד שעובד

02/08/2023

קוד הבדיקה הבא ב PyTest עובד מעולה (גם אם לא ממש ברור למה הוא טוב כי זו בדיקה מומצאת רק בשביל הפוסט):

from unittest.mock import MagicMock

def test_mock(monkeypatch):
    hello = MagicMock()
    fake_len = MagicMock()
    fake_len.return_value = 8
    hello.__len__ = fake_len

    with monkeypatch.context() as m:
        m.setattr(hello, '__len__', fake_len)
        assert len(hello) == 8
        assert fake_len.called

הבדיקה לימדה אותנו שהפונקציה len של פייתון באמת קוראת למתודה __len__ של הדבר שהעבירו לה. אבל זה לא הדבר החשוב כאן. יותר מעניין לשים לב לגירסה הפשוטה יותר של הבדיקה שעושה בדיוק את אותו דבר:

def test_mock(monkeypatch):
    hello = MagicMock()
    hello.__len__ = MagicMock(return_value=8)

    monkeypatch.setattr(hello, '__len__', hello.__len__)
    assert len(hello) == 8
    assert hello.__len__.called

שני השיפורים בגירסה הפשוטה הם מחיקות: מחיקת המשתנה fake_len שלא היה בו צורך, ומחיקת השימוש ב context - שוב מנגנון שבהקשר של הבדיקה הנוכחית לא היה נדרש.

ובכל זאת כשאנחנו מסתכלים על שתי הגירסאות קל לדמיין שאלה שינויים לא חשובים, שהכל היה בסדר גם קודם ושעדיף להתעסק בכתיבת פיצ'רים חדשים במקום לתקן את הקיים. אני חושב שהמציאות קצת יותר מורכבת מהסיבות הבאות:

  1. בשביל לתקן אני צריך להבין מה עושה monkeypatch.context ומתי כן צריך להשתמש בו. רק להבין את זה שווה את המאמץ, בלי קשר לקוד שיתקבל.

  2. קוד גרוע משתכפל ומייצר Cargo Cults - לאורך זמן אני יכול לדמיין עשרות ומאות בדיקות שישתמשו ב context, אפילו שאין בזה שום צורך. זה מבזבז זמן ובמיוחד אנחנו רואים את זה ב Code Reviews כשמתכנתת חדשה נכנסת לצוות וצריכה ללמוד את כל החוקים המשונים של הצוות שאין להם חשיבות טכנית.

  3. יום אחד כשיהיה באג יהיה יותר קל לחקור את הבעיה כשהקוד מורכב מפחות מבנים וכאלה שאני מבין טוב יותר.

לאורך זמן קוד מינימליסטי מנצח. אם התיקון קטן, אם יש גיט ובדיקות ואם אתם מבינים למה הקוד המוזר הגיע לשם (אולי אפילו מזהים מאיזה פוסט בסטאק אוברפלו הוא הועתק), שווה למחוק אותו.