הקוד הכי גרוע שכתבתי
היום הוא שוב תקף אותי, חוזר כאילו משום מקום כשכבר חשבתי שקברתי אותו לנצח - הקוד הכי גרוע שכתבתי. ככה הוא נראה בתוך 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. "מי יודע מה יישבר אם תיגע בזה עכשיו" אני אומר לעצמי, ומשאיר את השם המבלבל רק בשביל לשכוח איך להשתמש בו בפעם הבאה שאצטרך.