DDoS Community

Elad Sivan
Elad Sivan

Posted on

ביקורת קוד בקצרה - פרק 1

ואז חשבתי לעצמי, אני גם ככה משקיע זמן בלכתוב code review למה שלא נעביר אותו הלאה?
אז נקודה שהעלאתי בביקורת קוד האחרונה שלי:

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

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

שמרתי את זה קצר,
כמו פונקציה קריאה ☺️

Discussion (2)

Collapse
elhananavihail profile image
‫Elhanan Avihail • Edited on

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

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

נראה לי שרק שני מצבים עקרוניים בהם לא כדאי להוציא לפונקציה נתונים ברמת הפשטה נמוכה יותר:
א. שם הפונקציה יוצא זהה כמעט למימוש שלה.
למשל, getChildren שמקבלת אובייקט ומחזירה את מערך הילדים ששמור בפרופרטי children, זה באמת מיותר.
אבל אם היא אמורה להחזיר איזשהו ערך מחושב או מתוקן למשל getActiveChildren, שמחזירה את רק ילדים מסוימים בהתאם לערך active פנימי של כל ילד, היא מעולה. פשוט למה לא להוציא את המהלך הקטן הזה לפונקציה? למה להוסיף עוד סיבוב קטן במעבד של המוח בזמן קריאה?

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

Collapse
yc profile image
Y~C

מעניין, הנטיה שלי בד"כ זה לפרק הכל הכל לפונקציות.
אני מבין שזו לא תמיד גישה נכונה.