פשוט קוד: טיפול בשגיאות

אני עומד לעסוק בנושא בסיסי מאוד: Exception Handling. בסיסי - אך שיש מה לעסוק בו.

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

הערה: אני מניח שלא מדובר במערכת Embedded  או Realtime שם נמנעים מ Exception בשל העלות של stack unwinding. בשפות / מערכות אחרות (כמו Golang) - החליטו לא להשתמש ב Exceptions על מנת להיות צפויים ומבוקרים יותר. למשל: כאשר Exception נזרק ממקבץ של 4 שורות קוד - אני לא יודע בוודאות איזו שורה זרקה את ה Exception, ואולי אף איני לצפות את ה Exception שעלול להיזרק מכל שורה - מה שיוביל אותי לטיפול כללי ופחות מדויק בחריגה.

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

• שפת ג'אווה מתייחסת בשונה ל Checked Exceptions (היורשות מהמחלקה Exception) - אשר יש חובה קוד להתייחס אליהן, ו Unchecked Exceptions (היורשות מהמחלקה RuntimeException) המתנהגות כמו Exceptions ברוב שפות התכנות, ואפשר להתייחס אליהן - או להתעלם ואז הן יעברו לטיפול מי שקרא לי.
• הרעיון החדשני והמעניין של Checked Exceptions (המושפע מרעיון בשם checkers בשפת OCaml) הוכר בפועל כהחמצה, ולא הועתק (למיטב ידיעתי) לשום שפת תכנות נפוצה אחרת.
• גם בשפת קוטלין "ירדו" מהרעיון, ואין צורך להתייחס אחרת לחריגות היורשות מהמחלקה Exception.
• Error הוא ענף אחר בהיררכיית ההורשה, ושגיאה (Error) שתיזרק לא תיתפס כחריגה (Exception) - כי היא מחלקה מטיפוס אחר. ה Error נשמר למצבים חמורים שהאפליקציה לא אמורה לטפל בהם - ובעיקר נזרקים ע"י ה JVM (כגון OutOfMemoryError). לא זכור לי שאי פעם נתקלתי ב Error בפרודקשיין, בטח לא כזה שקוד היה יכול למנוע / להתמודד איתו.
• IntellJ (ה IDE) מימש משפטי TODO (כאלו שלא נכתבים כהערה) ככאלו שיזרקו Error. אם שכחתם לכתוב את הקוד שהתכוונתם אליו - ואתם מריצים את הקוד, אנחנו לא רוצים שהפרט הזה ייתפס כ Exception ויכתב לשורה ללוג אשר קל לפספס. הנה שימוש נכון ל Error.

חזרה לסיפור שלנו: אז איזה סוג של Exception כדאי לזרוק?

• ברור לי שזה לא Error. (למרות שנתקלתי במקרים בהם אנשים זרקו Errors ב flows אפליקטיביים).
• אני לא מתעסק בהבחנה בין Checked ל Unchecked Exception - וטוב שכך.
• יש מן עצה כזו שאומרת "Favor the use of standard exceptions", אך עדיין נותר לי מבחר גדול של Exceptions רק מתוך הספריות הסטנדרטיות של ג'אווה:

הרשימה באמת, היא אפילו יותר ארוכה...

אני רוצה לכתוב קוד טוב יותר וקריא יותר - אבל איך בוחרים?

צעד הגיוני ונפוץ יחסית, הוא ללכת ולהתייעץ בכל מיני "מילונים" לסוגי החריגות השונות, ומתי להשתמש בהן:

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

• יש כאלו שיבחרו ב IllegalArgumentException - כי הפעילו את המתודה עם ארגומנט לא חוקי.
• יש כאלו שיבחרו ב IllegalStateException - כי בכלל ארגומנט לא חוקי לא אמור להגיע כך ללב המערכת. זו תקלה במצב המערכת. "אם הגענו עד לכאן - זו כבר תקלה ב state".
• יש כאלו שיבחרו ב NotFoundException - באמת לא מצאתי את הלקוח הזה.
• יש כאלו שיבחרו ב UnsupportedOperationException - זה לא חוקי לשלוף פרטי לקוח עם מזהה לא נכון (?!).
• יש כאלו שפחות מתאמצים ופשוט זורקים RuntimeException (החריגה ארעה בזמן ריצה) או פשוט סתם Exception.
• יש כאלו שיותר מתאמצים ומגדירים טיפוס חדש של חריגה למצב הספציפי, למשל: CustomerNotFoundByIdException. 
• אפילו ראיתי מקרה בו בתסריט כזה נזרק SecurityException. אני מניח שטיפול ב id שגוי של משתמש הוא גם בעייתי בהיבטי האבטחה של המערכת.

אם מפתחים שונים בוחרים חריגות מטיפוסים שונים לאותו המצב, אז:

1. הקוד שלנו לא ממש אחיד => מה שמפחית במעט את קריאות הקוד.
2. אם אני רוצה לתפוס את המקרה כ Exception - אני לא באמת יודע לאיזה Exception לצפות, ויותר גרוע: אני יכול לתפוס את סוג ה Exception - שנזרק ע"י קטע קוד אחר שהמתכנת שלו חשב שזה טיפוס ה Exception הנכון ביותר לתאר אותו.

אם חשבתם אולי לשלוח "שק" של Exceptions שיתאר בצורה רחבה יותר את מה שקרה - אנא חשבו על זה שוב. זה רעיון לא מוצלח....

>> אז מה עושים?

כיצד אתם בוחרים באיזה סוג Exception להשתמש?

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

בחזרה למהות

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

אנו זורקים Exceptions כיוון שמשהו השתבש, ואנחנו לא יכולים, או לא נכון לנו להמשיך את ה flow. למשל: אם אנו מגלים שאנו עומדים ליצור חוסר-עקביות (inconsistency) בבסיס-הנתונים, כנראה שעדיף לנו לעצור עכשיו מאשר ליצור בעיה עתידית, קשה יותר לזיהוי ולתיקון.

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


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

1. Rollback של טרנזקציה / פעולה, או פעולות cleanup - זה קורה לפעמים.
2. כתיבת הודעת לוג (שגיאה / אזהרה) - בכדי ללמוד עליה ולטפל ידנית במצב ו/או לשפר את המערכת שמצב זה לא יקרה בשנית. את זה עושים כמעט תמיד.
3. הצגת אינדיקציה למשתמש-הקצה ב UI, ברמת פירוט כזו או אחרת, שמשהו השתבש - אם קיים משתמש כזה.

זהו. זו בגדול התוצאה.

ברוב המקרים - הטיפול האמיתי ב Exception יהיה מעבר ל Flow בקוד:

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

הנה קוד לדוגמה שתופס Exception:

try { ... } catch (e: Exception) {   logger.error("something went wrong $e") }

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


היי, רגע!

אני מקווה ששמתם לב שמשהו מאוד לא בסדר בדוגמת הקוד שהרגע סיפקתי. כמה משהו:

• לא שלחנו את ה exception עצמו ל logger. זה אומר שלא יודפס ה stack-trace - וזו בעיה חמורה, כי יהיה קשה מאוד להבין מה השתבש בחקירה שתגיע מאוחר יותר.
• אני מבקש בזאת מכותבי ה logging framework הבא לג'אווה שהחתימה למתודה (...)logger.error תחייב לספק ארגומנט מסוג ?Exception. השכחה לשלוח את ה Exception ללוגר היא מספיק מזיקה. אם אנו רוצים לכתוב ללוג error שלא קשור ל Exception - אני מעדיף שנשלח ערך null.
• בלענו את ה Exception. האם זו הייתה הכוונה?
• לפעמים כן - וזה בסדר. מספיק שכתבנו ללוג (אבל נוסח ההודעה יהיה יותר כמו "failed to do...") ומישהו יטפל בזה אח"כ.
• לפעמים לא - וזו יכולה להיות ממש תקלה. 
• גם במקרה כזה הייתי שמח אם ברירת המחדל של השפה הייתה להמשיך ולזרוק את השגיאה, עם אפשרות להוסיף משפט break אולי swallow או משהו - במקרים בהם לא רוצים במודע להמשיך לזרוק את השגיאה.
• הדפסנו את האובייקט של ה Exception - ולא את הודעת השגיאה (e.message).
• ברוב הפעמים, מימוש ברירת-המחדל ל ()toString של מחלקת ה Exception יהיה message - ואז הכל בסדר.
• במקרים בהם המימוש הוא אחר, למשל: כתובת האובייקט בזיכרון - איבדנו את ההזדמנות לאסוף מידע שימושי לגבי השגיאה.

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

fun doA() {   try {   ...   doB()   ...   } catch (e: Exception) {     logger.error("something went wrong: $e.message", e)     throw e   } } fun doB() {   try {   ...   doD()   ...   } catch (e: Exception) {     logger.error("Dubi: something din't work as expected: $e.message", e)     throw e   } } fun doD() {   try {   ...   } catch (e: Exception) {     logger.error("doD: WTF?! $e.message", e)     throw e   } }

קרתה שגיאה אחת, אבל בלוג יכתבו 3 stack trances שעשויים להראות כמו 3 שגיאות. ה stack traces הם כמעט חופפים, כאשר בכל פעם נוספת רק עוד שורה אחת.
אפשר בקלות לבזבז זמן בחקירה של הלוג שנכתב ע"י דודי (כלומר: doD) או הלוג שנכתב ע"י דובי (doB) - בעוד חסר לנו ההקשר החשוב שהתחלנו לפעול בעצם מ doA.

בגדול:

• ב catch clause בחרו: או שאתם מדפיסים הודעה ללוג - או שאתם זורקים שגיאה הלאה.
• כל Framework / WebServer שמכבד את עצמו יתפוס את כל השגיאות שהתרחשו - ויכתוב אותן ללוג.
• כתיבת ה Stack trace ללוג ברמה הגבוהה ביותר - תספק את מירב המידע, ולכן זו השגיאה שאנו רוצים לחקור. חבל לכתוב הודעות כפולות ללוג.
• הערת משנה: אם יש stack trace אז יהיה כתוב איזה פונקציה נקראה. עדיין המון אנשים אוהבים להוסיף בעצמם את שם המתודה (ולפעמים גם את שם המחלקה) להודעת השגיאה - וזה דיי מיותר. נחשו מה קורה כאשר עושים rename לשם המתודה?
• בהודעת השגיאה חשוב לציין מידע חדש ומעניין. למשל: פרמטרים מסוימים של הריצה - שלא תהיה לנו גישה אליהם מאוחר יותר.

הנה דוגמת הקוד הזו בגרסה הטובה יותר שלה:

fun doA() {   try {   ...   doB()   ...   } catch (e: Exception) {     throw Exception("failed ... id=$id", e) // = java's "new Exception"   } } fun doB() {   ...   doD()   ...   // if you have nothing smart/new to say - spare our time. } fun doD() {   try {   ...   } catch (e: Exception) {     throw Exception("Failed DoDing: x=$x, y=$y", e)   } }

ה Framework הוא זה שיתפוס את ה Exception ויכתוב אותה, ואת ה stack trace המלא - ללוג. זה ה Best Practice שנקרא global exception handling.

שווה לציין, שב Web Frameworks ההתנהגות המקובלת היא שה Framework תופס את ה Exception ומוציא החוצה הודעה לקונית בנוסח "HTTP 500 internal server error". אנחנו לא רוצים לשלוח ברשת payload גדול של כל ה stacktrace, ומבחינת אבטחה אנחנו לא רוצים לחשוף החוצה את ה internal של המערכת שלנו וללמד את התוקף הפוטנציאלי מה קורה. לכן, כברירת-מחדל, הודעות השגיאה ששקדתם עליהן יגיעו ללוג - ולא ללקוח.


בואו נחזור לשאלה האחרונה ששאלנו:

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

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

התשובה

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

מי הלקוח שעומד לתפוס את החריגה מהטיפוס המסוים, ומה הוא עומד לעשות עם המידע הנוסף שהטיפוס הזה סיפק לו?

זו שאלת המפתח, שלצערי כמעט ולא נשאלת.

אם מי שעומד לתפוס את ה Exception הוא ה Framework בכדי לכתוב אותו לוג - אין כל טעם לשגר Exception מטיפוס מיוחד.
אם מי שעומד לתפוס את ה Exception הוא קוד, בכדי לבצע באופן גנרי rollback / resource cleanup - אין כל טעם לשגר Exception מסוג מיוחד.

המצב היחידי שבו יש טעם לשגר שגיאה מסוג מסוים - הוא אם מישהו יחכה לסוג הספציפי של ה Exception.

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

אני אצטט לרגע אנשים שחקרו את הנושא עמוק ממני:

"Examination of small programs leads to the conclusion that requiring exception specifications could both enhance developer productivity and enhance code quality, but experience with large software projects suggests a different result – decreased productivity and little or no increase in code quality." -- Joseph R. Kiniry

ובכן זה התסריט המשמעותי היחיד לשימוש בטיפוס ספציפי של Exception:

try { ... } catch (e: CustomException) { // do some cleanup / revert logic } catch (e: Exception) {   throw Exception("Failed DoDing: x=$x, y=$y", e) }

כאשר קוד מתייחס לטיפוס הזה, ומריץ איתו branching אחר של הקוד.

חשוב לציין ש cleanup logic צריך ברוב הפעמים לשבת ב finally clause - ולרוץ ללא קשר לסוג התקלה שהתרחשה.

התקדמנו.

במקרה המסוים שלקוד אכפת הטיפוס של ה Exception, באיזה טיפוס כדאי להשתמש? מהו בעצם ה CustomException?

נחזור רגע להמלצה המוכרת: "Favor the use of standard exceptions".
שימו לב שאם CustomException יהיה Exception סטנדרטי כמו IllegalArgumentException אזי הוא יוכל להיזרק לא רק מהקוד שלנו - אלא מכל קוד אחר במערכת שבחר "להשתמש ב Standard Exceptions". אולי אלו 3rd Party, אולי גם הספריות הסטנדרטיות של ג'אווה.

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


לכן: כאשר אתם רוצים לתאר מצב יוצא דופן, שיוביל לטיפול מיוחד - הגדירו מחלקה חדשה של Custom Exception, או לפחות מחלקה ידועה, בה אתם משתמשים במזהה מיוחד (למשל: קוד שגיאה ייחודי).


מתי, אם כן, אפשר להשתמש ב Standard Exception?

• כאשר אתם רוצים לייצר הבחנה בין שגיאות שונות של הפונקציה שלכם. אם יש רק מצב-שגיאה אחד משמעותי - מספיק להשתמש פשוט ב Runtime)Exception).
• חשוב מאוד לתעד מה המשמעות של כל Exception ספציפי שנזרק. בלי תיעוד - זה תרגיל ב fuzzy human communication ביניכם למי שעומד לכתוב את הקוד שיטפל בשגיאה.
• כאשר קהל היעד שלכם לא ידוע (נניח: אתם כותבים ספריה סטנדרטית), ואתם לא יודעים איך המשתמשים עומדים להשתמש ב Exception שזרקתם , או שיש מגוון רחב של אפשרויות תגובה.
• כאשר אתם משוכנעים שאתם הם אלו שעומדים לזרוק את השגיאה הזו, ולא פונקציה אחרת שאתם קוראים לה!

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

השאלה היותר חשובה, היא מתי לתפוס Standard Exceptions?

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

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

int day = Instant.now().get(ChronoField.DAY_OF_MONTH);

יזרוק חריגה "סטנדרטית" (כי הקוד הוא של ג'אווה [א]) מסוג UnsupportedTemporalTypeException.

למה? בכדי להדגיש ש Instant (המייצג של epoch ב Java Time APIs) נועד לשימוש ע"י מכונה, ולא בכדי לייצג מידע על תאריך שיגיע למשתמשים בני-אדם.

האם זה נכון, לזרוק שגיאה בשל "שימוש שנראה שגוי בספריה"?

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


אני רצה לסיים ולציין תנאים מומלצים לשימוש ב Standard Exception:

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

דוגמה אחת היא javax.ws.rs.WebApplicationException: 

1. היא אמנם "סטנדרטית בתעשייה" ולא ייחודית לכם. לכן, נכון לזרוק אותה - אבל כדאי להימנע ולהסתמך עליה בלכידת Exception לצרכים אפליקטיביים. היא עשויה להיזרק ע"י כל אחד.
2. יש לה סמנטיקה משמעותית וברורה ברמת המכונה: Web Applications Servers יתפסו אותה ברמתם - אבל יעבירו את הודעת השגיאה שהוגדרה בה, כמו שהיא, ללקוח. אפשר גם לציין במחלקה הזו גם HTTP Status קוד שיעבור גם הוא ללקוח. למשל: HTTP 403 - הוא קוד בעל משמעות חשובה.
1. מי שזורק את WebApplicationException, חשוב שיבין את הסמנטיקה שלה - ולא יכתוב בה הודעת שגיאה עם מידע פנימי שלא אמור להגיע ללקוח שביצע את הקריאה.

בדוגמה אחרת, הגדרנו ב Next-Insurance טיפוס Exception פנימי בשם RobinUserInputException.

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

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

סיכום

אני מקווה שהצלחנו להאיר ולהדגים כמה עקרונות חשובים לגבי Exception Handling.

לצערי, הרבה חשיבה מסביב ל Exception Handling מוקדשת ל Tooling (מחלקות שונות של Exceptions) - ולא למהות השגיאה, וכיצד לטפל בה.

חדי העין אולי שמו לב שההמלצה שסתרתי בפוסט, "Favor the use of standard exceptions" - מגיעה מספר מכובד (של מחבר מכובד) בשם Effective Java. הנה סיכום הדברים:

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

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

שיהיה בהצלחה!


-----

[א] אני קצת צוחק, כי זו באמת חריגה ממוקדת, שלא משתמשים בה מחוץ ל Java Time API - אבל יש כאלו שרואים כל חריגה של הספריות של ג'אווה כחריגות "סטנדרטיות".

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

ללמוד מהקוד

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

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

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

את קטע הקוד הבא מצאתי בתשובה של StackOverflow, תשובה שזכתה לכמה upvotes [א].

אז בואו נתחיל.

הקוד

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

fun balance(amount: Int, round: Boolean, rate: Double, year: Int) : Double {   var yearlyInterestPaid: Double   var totalAmount = amount.toDouble()   for (i in 0..year) { // for (int i = 0; i <= year; i++ ){       yearlyInterestPaid = totalAmount * rate       if (round) yearlyInterestPaid = Math.floor(yearlyInterestPaid * 100) / 100 .       totalAmount += yearlyInterestPaid   }   return totalAmount }

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



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

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

בעיות בולטות במיוחד תופסות את עינינו - ולעתים מסיטות את תשומת הלב שלנו מבעיות חמורות יותר.

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

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

מבט שני

נחזור ונתבונן בקוד, לאחר שהסרנו את הבעיה הבולטת:

fun calcInterest(amount: Int, round: Boolean, rate: Double, year: Int) : Double {   var yearlyInterestPaid: Double   var totalAmount = amount.toDouble()   for (i in 0..year) {       yearlyInterestPaid = totalAmount * rate       if (round) yearlyInterestPaid = Math.floor(yearlyInterestPaid * 100) / 100 .       totalAmount += yearlyInterestPaid   }   return totalAmount}

האם אתם מזהים עכשיו בעיות נוספות?


----


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

הנה הקוד לאחר שני שיפורים נוספים:

fun calcInterest(amount: Double,                 round: Boolean,                 rate: Double,                 year: Int) : Double {     var totalAmount = amount     for (i in 0..year) {       var yearlyInterestPaid = totalAmount * rate       if (round) yearlyInterestPaid = Math.floor(yearlyInterestPaid * 100) / 100 .       totalAmount += yearlyInterestPaid   }     return totalAmount }

אם לא שמתם לב, אז הנה התיקונים (הקטנים) שביצענו:

• הכנסנו את yearlyInterestPaid ל scope מצומצם יותר. אני אזכיר את הכלל הידוע: ״הגדירו משתנים מאוחר ככל האפשר / קרוב ביותר למקום שמשתמשים בהם״.
• מדוע אנו מבצעים המרת טיפוס ל totalAmount? יכול להיות שאין במערכת צורך מיידי בכך, אך הרבה יותר הגיוני שפעולה מתמטית תתבצע על אותו הטיפוס: אם ערך ההחזרה של ה amount לאחר הריבית הוא Double - אז גם שהקלט של ערך הבסיס יהיה מהטיפוס הזה.

מה עוד? השארנו את הדברים הכבדים לסוף.

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

ובכן, כיצד נראית קריאה לפונקציה שלנו?

calcInterest(1000.0, true, 0.04, 10);

מה ניתן להבין מהשורה הזו?

קצת קשה להבין בדיוק מה קורה כאן. מה true? מה בדיוק אומר כל מספר?
האם ייתכן והערך true נוסך בנו יותר בטחון בשימוש בפונקציה מהערך false?! :-)

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

יש IDEs שיוספו כ hint את שמות המשתנים ליד כל ערך - אבל שימו לב ששמות המשתנים לא פותרים את הבעיה:

calcInterest(amount: 1000.0, round: true, rate: 0.04, year: 10);

אם בכדי להבין מה שורה עושה, עלינו להיכנס לקוד הפונקציה - זה לא מצב טוב. בזבזנו זמן, וזו אינדיקציה חזקה מאוד שה "UX של הפונקציה״ (אפשר לקרוא לזה DX = Developer eXperience) - אינה טובה.

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

שיפור ה DX של הפונקציה שלנו

בואו נהפוך את חתימת הפונקציה שלנו לקריאה יותר, וכך - למובנת יותר.

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

כלל אצבע מוכר, הוא לא לשלוח יותר מ 4 פרמטרים לפונקציה. לעתים אנשים חושבים שהבעיה היא חתימה ארוכה של הפונקציה - ואז מעבירים את 12 הפרמטרים ל Request Object אחר בן 12 פרמטרים.

בעצם - מה ההיגיון בזה?!

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

val totalSalary = managersSalary + employeeSalary

עכשיו הופך ל:

val totalSalary = request.managersSalary + request.employeeSalary

זהו עומס נוסף על הקוד, שלא הופך אותו קל יותר לקריאה.

במקום ליצור Request Object אחד, עדיף לייצר 3-4 Request Objects קטנים המקבצים פרמטרים דומים יחדיו.

כך הגישה ל request object היא לא מעמיסה - אלה גם נותנת משמעות לפרמטרים. למשל:

val totalSalary = salaries.managers + salaries.employee

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

כלומר: ב Request Objects טובים יעשה שימוש חוזר - שיפחית את העומס על הקוד.

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

סדר הגיוני של פרמטרים: 

• קיבוץ פרמטרים דומים בזה אחר זה.
• הצבת הפרמטרים החשובים בתחילת הרשימה - והחשובים פחות בסופה.
• אנו רוצים להתחיל לקרוא את חתימת הפונקציה - מהעיקר אל הטפל.

למי שעובד ב IntelliJ, הקיצור CMD+F6 (במק) - הוא קיצור חשוב מאוד. כשאתם מזהים סדר לא נהיר של פרמטרים - הקדישו חצי-דקה וסדרו אותם. IDE מודרני באמת מאפשר לבצע שינוי כזה במהירות ובבטיחות.

הנה הקוד שלנו לאחר השיפור:

fun calcInterest(initialAmount: Double,                 interestRate: Double,                 years: Int,                 roundInterestYearly: Boolean): Double {   var totalAmount = initialAmount   for (i in 0..years) {       var yearlyInterestPaid = totalAmount * interestRate       if (roundInterestYearly) yearlyInterestPaid = floor(yearlyInterestPaid * 100) / 100       totalAmount += yearlyInterestPaid   }   return totalAmount }

שינוי פעוט בשם פרמטר, מ year ל years - הוא משמעותי.

round הוא שם נורא בעיני. הפכנו אותו לשם בעל משמעות.

ההעברה של round לסוף רשימת הפרמטרים מבליטה שהוא:

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

לא סיימנו!

הקוד, אם קיבלנו אותו ל Review לא כולל חלק חיוני - בדיקות יחידה.

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

שאלות מהותיות הן:

• האם אתם מאשרים PR כאשר פונקציה עם Pure Business Logic מוצגת - ללא בדיקת יחידה?
• האם אתם מאשרים PR אם קוד לוגי הוא קרוב להיות טהור, ובמעט עבודה אפשר לזקק אותו - ולבדוק אותו בבדיקות יחידה?

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

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

לא כתבתי בדיקות יחידה לקוד (אני לרוב כותב, אם אני מפרסם קוד ב SO - כך צריך), אבל מהרצה פשוטה של הקוד ב IDE זיהיתי כבר שני מקרים שעלולים להיות בעייתיים, ובלתי צפויים:

• הפעלת הפונקציה עם סכום של 1000, בריבית הנקובה למעלה, לאורך עשר שנים - מחזירה תוצאה של 1539.4540563150777.
• כאשר אנו מבצעים עיגול כל שנה - הערך הוא עגול (1539.41), אך בהרצה לשש שנים - הערך המתקבל עם הרצת עיגול הוא 1315.9099999999999.
• סכום חיובי וסכום שלילי - לא מתעגלים בצורה סימטרית (כי ה floor הוא אבסולוטי לערך).

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

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


אם אתם מופתעים, ושואלים את עצמכם ״אבל... מדובר ב Stack Overflow - אתר עם ביקורת רבה וקשוחה, איך זה יכול להיות?!״ אני אתן את הציטוט הבא שקראתי בו לאחרונה:

The research echoes an academic paper from 2017 that found 1,161 insecure code snippets posted on Stack Overflow had been copied and pasted into 1.3m Android applications available on Google Play. -- source

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

לא פעם יצא לי למצוא קוד מורכב / מאופטם בצורה חריגה לסביבה שלו - וחיפוש קצר בגוגל הראה שהוא הועתק, as-is, מ StackOverflow. אנחנו עושים את זה, ולא מעט. כולנו במידה כזו או אחרת ״Google Engineers״ (= כאלו שמחפשים בגוגל ואז מעתיקים את התשובה) - אבל חשוב לבדוק את הקוד ולא לסמוך עליו בעיניים עצומות.

סיכום

אשמח לדעת מה דעתכם.

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

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



----

[א] ליתר דיוק: שילבתי שתי תשובות שונות שזכו ל upvotes - על מנת לחדד כמה נקודות לדיון.