לחץ "Enter" למעבר לתוכן

כשהקוד נראה לא ברור

0

טוב, מה בדיוק הוא רוצה להגיד בזה?
כולם יודעים שקוד צריך להיות ברור.
קוד לא ברור – מה יש לעשות איתו בכלל?

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

במילים "לא ברור", הכוונה היא לא לתוכן של הקוד.
הכוונה היא ממש לצורה הויזואלית שלו.
איך הוא כתוב, ולא מה כתוב בו.

Code Smells

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

למי שנמצא מספיק זמן בתעשיית התוכנה, המושג Code-Smell כנראה אינו חדש. למי שלא, המושג פשוט למדי: code-smell מתאר אינדיקציה אפשרית לבעייה בקוד או בדיזיין. לא מדובר בבאג, וגם לא בהכרח בהחלטת קידוד גרועה. מדובר בתופעה המעידה על כך שדבר מה דורש מחשבה שנייה, ואולי גם רפקטורינג, כיוון שלעתים זהו סימן לבעייה עמוקה יותר.

קוד לא ברור

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

 

Out of focus
יוצא מפוקוס. רובין וויליאמס ב"לפרק את הארי".

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

 

נסו זאת בעצמכם!

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

 

הצלחת?
אילו מהם נראים כמו קוד מסודר ונכון?
זה מה שאני חושב כאשר אני נתקל בקוד המזכיר את המבנים המופיעים כאן.

קוד מס. 4

נתחיל בדוגמא הפשוטה ביותר:

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

ולמה, בעצם, צריך בכלל שורות ארוכות כל כך?
סיכוי לא רע שמסתתרות מאחור בעיות נוספות, למשל:

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

הדוגמאות האלה, כשלעצמן, הן סימנים מעידים לאי הקפדה על עקרונות ה-SOLID. בעיקר – SRP, עקרון האחריות היחידה (Single Responsibility Principle) ו-OCP, עקרון הקוד הפתוח/סגור (Open/Close Principle).

בעיות אפשריות: קריאות, תחזוקתיות, SRP, OCP.

קוד מס. 8

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

בעיות אפשריות: קריאות, תחזוקתיות, SRP, OCP.

קוד מס. 2

אמנם, לא צפוף כמו הדוגמא הקודמת, אבל משהו אחר מעיק על הקריאה: מישהו כאן השתמש הרבה ב-CAPS-LOCK.
כמו בכל טקסט שהוא, כאשר ישנן A LOT OF CAPITAL LETTERS, זה מטריד את העיניים ומקשה על הקריאה. אמנם, בשפות פיתוח ישנן סיבות טובות ומוצדקות לשימוש באותיות כאלה, למשל לציון קבועים או ביטויי מאקרו. מצד שני, זה עדיין לא הופך אותם לנוחים לקריאה.
לכן, ייתכן בהחלט שבמקרה הזה השימוש מוצדק ונדרש. בכל זאת, כדאי לבחון את הקוד ולראות האם ניתן לשכתב אותו באופן כזה שיקל על הקריאה. למשל, שימוש בשמות קצרים יותר, פיצול למספר שורות, החלפה בקריאה לפונקציה וכדומה.

בעיות אפשריות: קריאות.

קוד מס. 1

הקוד הזה נראה כמו סוג של בחירה בין אפשרויות מרובות, מהסוג של switch-case או if-else-if.

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

במקרה הספציפי הזה, על כל פנים, ישנו חלק חשוד במיוחד.
רוב ה"אפשרויות" בקוד דומות זו לזו: שורה אחת של תנאי, שורה אחת של פעולה.
אבל מה קורה בשורות 20–25? הן שוברות את המבנה הזה. אולי יש שם פיצול לעוד if/else או switch/case. אולי יש שם לוגיקה יותר מורכבת מאשר פעולה יחידה כמו בשורות הקודמות. כך או כך, כמעט בוודאות השורות האלה אינן אמורות להיות שם. הן ככל הנראה מקרה של רמה שונה של אבסטרקציה (different level of abstraction) ביחס לשאר הקוד, שבעצמו הוא הפרה של עקרון האחריות היחידה (Single Responsibility Principle).

בעיות אפשריות: SRP, OCP.

קוד מס. 3

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

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

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

אבל… זה לא הכל.
גם בשורה 5 וגם בשורה 10 אנחנו רואים התחלה של קינון מוגזם מהסוג המדובר.
כך שעקרון ה-SRP נשבר כמעט בוודאות. צריך סיבות טובות מאוד כדי לשמור על שני החלקים בתוך אותה הפונקציה.

שורות 21 ו-26 אפילו יותר בעייתיות: הן מפצלות נתיב שגם כך כבר נמצא ברמת קינון עמוקה מדי לשני נתיבים שונים. אם בבוא היום מישהו יצטרך לשנות את ההתנהגות או לתקן באג בפונקציה כזו, ייקח הרבה מאוד זמן כדי להבין מה קורה שם וכדי לוודא שלא נעשתה טעות והדברים נכנסו למקום הנכון. ותכל'ס — רוב הסיכויים הם שלא.

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

בעיות אפשריות: קריאות, תחזוקתיות, SRP, OCP.

קוד מס. 5

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

בעיות אפשריות: קריאות.

קוד מס. 6

מזהה את המבנה הזה?
אני מקווה מאוד שלא, אבל אם יש לך קילומטראז' מספיק בעולם התוכנה, נתקלת בזה כמעט בוודאות.
אולי לטובת ערכים של enum, אולי לטיפוסים ולשמות שבתוך struct כלשהו. אולי לחלוקה בין מפתחות וערכים באיזו map או dictionary.
השימוש במבנה של שתי עמודות היה בעבר מקובל מאוד. למרבה השמחה, היום הוא כבר פחות מקובל, ויש לזה סיבה טובה.

הבלוק הראשון בקוד מציג בצורה משכנעת מאוד "למה לא". רוב הקוד מסודר בשתי עמודות. אבל, שתי שורות (8 ו-11) שוברות את המבנה הזה. מן הסתם, זו תוצאה של עדכון משלב מאוחר יותר בקוד, שלרוע המזל נאלץ להשתמש בשמות ארוכים מהמתוכנן. אז אולי כשכתבו את הקוד הזה לראשונה ולאורך זמן מה הוא היה קריא, אך לא עוד.

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

בעיות אפשריות: קריאות, תחזוקתיות.

קוד מס. 7

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

  • שימוש בשמות ארוכים מדי
  • פונקציות שמקבלות יותר מדי ארגומנטים
  • לוגיקה מסובכת או ביטויים מורכבים מדי (למשל, משפטי תנאי)

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

בעיות אפשריות: קריאות, תחזוקתיות, SRP, OCP.

קוד מס. 9

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

לסיכום

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

יש דוגמאות נוספות שבהן נתקלת? אשמח לראות אותן!