ככה לא כותבים בדיקות תקינות
בימים האחרונים קיבלתי הרבה תגובות מחזקות על ההחלטה לשנות את השעה של המייל האוטומטי בשבתות כך שישלח במוצאי שבת. זה שלח אותי לחפש גם רשימה נוחה של חגים כך שאפשר יהיה לעשות את אותו טריק גם שם.
התוצאה המעניינת הראשונה בגוגל היא פרויקט בשם hebcal שזה תוכנית C שיודעת לחשב ולהציג את תאריכי החגים אלפי שנים קדימה. כך לדוגמא:
$ hebcal 4 2059
4/1/2059 Pesach IV (CH''M)
4/2/2059 Pesach V (CH''M)
4/3/2059 Pesach VI (CH''M)
4/4/2059 Pesach VII
4/5/2059 Pesach VIII
4/10/2059 Yom HaShoah
4/13/2059 Rosh Chodesh Iyyar
4/14/2059 Rosh Chodesh Iyyar
4/16/2059 Yom HaZikaron
4/17/2059 Yom HaAtzma'ut
4/27/2059 Pesach Sheni
אבל תוך כדי משחקים עם הכלי נתקלתי בהתנהגות מוזרה. מסתבר ש hebcal נשברת כשמספר החודש גדול מ 12. כך למשל:
$ hebcal 99999 2059
Segmentation fault: 11
ויש גם קלטים שמכניסים אותה ללולאה אין סופית. בואו ניכנס לקוד לראות מה קרה כאן. את קוד הפרויקט המלא אפשר למצוא בגיטהאב בקישור:
https://github.com/hebcal/hebcal
הפונקציה הראשונה שצריכה להטריד אותנו היא זו שמופעלת כדי לפענח את הפרמטרים (הקובץ start.c) ושם אנחנו פוגשים את הקטע הבא:
startAbs = greg2abs(tempDate);
tempDate.dd = MonthLengths[LEAP(theYear)][theMonth];
endAbs = greg2abs(tempDate);
שכולל גישה ישירה למערך בשם MonthLengths לפי הערך של המשתנה theMonth. ומאיפה משתנה זה הגיע? נגלגל קצת למעלה כדי למצוא את:
theMonth = atoi(remain[0]);
הערך remain הוא מה שהגיע משורת הפקודה כלומר המספר 99999. אגב מעניין לציין שבדיקה האם החודש תקין כן מופיעה במקום אחר בתוכנית כך:
if (theMonth > 12)
die("The month must be less than 13", "");
אבל בגלל שהפונקציה הראשית בנויה כ switch/case גדול הבדיקה מופיעה רק בענף אחד של ה switch/case ולא באחרים, ולכן לא בודקת את המצב בו משתמש מציין שנה וגם חודש.
והנה קיבלתם דוגמא מעולה לאיך לא לכתוב בדיקת תקינות: בדיקת תקינות טובה צריכה להופיע כמה שיותר קרוב למקום המסוכן, כדי שהיא תופעל בכל מצב בו הקוד המסוכן עומד להיקרא. במקרה שלנו הקוד המסוכן הוא הקריאה הישירה מהזיכרון ולכן בדיקת התקינות היתה צריכה להופיע בצמוד לשורה:
tempDate.dd = MonthLengths[LEAP(theYear)][theMonth];
המשך שיטוט בקוד הראה שיש עוד מקום שניגש למערך הזה ולכן ההצעה שלי ליצור פונקציה לקריאה מהמערך והפונקציה תדאג גם לבדיקת התקינות. קוד הפונקציה יראה בערך כך:
int getMonthLength(int year, int month)
{
if (month < 0 || month > 13)
{
return 0;
}
return MonthLengths[LEAP (year)][month];
}
עכשיו אפשר יהיה לשנות את שאר הקוד בתוכנית כך שכל הגישות למערך MonthLengths יהיו דרך הפונקציה.
אפשר לראות את השינויים המלאים שהצעתי לפרויקט ב Pull Request שהעליתי כאן:
https://github.com/hebcal/hebcal/pull/157/files
והכלל בקיצור: כתבו בדיקות תקינות כמה שיותר קרוב לקוד המסוכן. כך תבטיחו שבכל שינוי עתידי בדיקות התקינות שלכם ימשיכו לעבוד.