היום הוא שוב תקף אותי, חוזר כאילו משום מקום כשכבר חשבתי שקברתי אותו לנצח - הקוד הכי גרוע שכתבתי. ככה הוא נראה בתוך Ruby Class שיורש מ ActiveRecord::Base:

def is_valid?
  return false if remaining && (remaining <= 0)

  now = Time.now.to_date
  now >= start_date && now <= end_date
end

אז מה בעצם כל כך גרוע בו? נכון, יש את הקריאה ל Time.now שמתעלמת מאזור זמן, יש מבנה קצת מוזר של return בהתחלה והסתמכות על התוצאה של הביטוי האחרון בתור ערך החזר בסוף, אבל שום דבר מזה הוא לא אסון גדול.

מה שהופך את הקוד הזה לגרוע הוא ההתנגשות עם הקונבנציה שמקיפה אותו - ב Active Record של ריילס יש כבר פונקציה מובנית בשם valid? (בלי ה is) שבודקת אם אוביקט הוא וולידי. השם שאני בחרתי, is_valid? כדי לבדוק משהו אחר לגמרי, מבלבל אותי כל פעם מחדש. זו דוגמה מתוך התיעוד של ריילס לבניית וולידציה מותאמת אישית למחלקה:

class Invoice < ApplicationRecord
  validate :expiration_date_cannot_be_in_the_past,
    :discount_cannot_be_greater_than_total_value

  def expiration_date_cannot_be_in_the_past
    if expiration_date.present? && expiration_date < Date.today
      errors.add(:expiration_date, "can't be in the past")
    end
  end

  def discount_cannot_be_greater_than_total_value
    if discount > total_value
      errors.add(:discount, "can't be greater than total value")
    end
  end
end

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

הפונקציה שלי, למרות שמה המבלבל, בכלל לא קשורה למידע שמשתמשים מכניסים מטפסים אלא לזמן. ערך ההחזר שלה ישתנה כשהזמן יעבור והתאריך יהיה מתקדם יותר מ end_date של האוביקט. שמות כמו active או usable או אפילו להפוך את הפונקציה ולקרוא לה expired היו הופכים את החיים שלי להרבה יותר קלים.

ומה שהופך את הקוד ל"הכי גרוע", זה ששם של פונקציה אף פעם לא מרגיש מספיק חשוב בשביל לשנות אותו ב Refactoring. "מי יודע מה יישבר אם תיגע בזה עכשיו" אני אומר לעצמי, ומשאיר את השם המבלבל רק בשביל לשכוח איך להשתמש בו בפעם הבאה שאצטרך.