רוטר - עזרה עם code review למשהו שכתבתי

יומן פעילות	לוח שנה	מבזקי חדשות	תקנון	RSS	כותרות	כותרות לפי תגובה	ניקוי קוקיז	IP	להורדת אפליקציה
eBay	AliExpress	GearBest	Amazon	Booking	Kiwi	SkyScanner	Trip Advisor

"עזרה עם code review למשהו שכתבתי"

גירסת הדפסה

קבוצות דיון פיתוח, תיכנות ובניית אתרים נושא #22324	מנהל סגן המנהל מפקח Winner צל"ש מומחה

אשכול מספר 22324

כמה עוד

חבר מתאריך 18.11.09
234 הודעות, 11 פידבק

10:04 15.05.19

עזרה עם code review למשהו שכתבתי

אני כרגע עושה פרויקט שאני המפתח היחיד בצוות, אז אין לי את מי לשאול כל כך.
רציתי לבקש את עזרתכם איך לשפר את הקטע קוד הבא-
הבנתי שעדיף לא לעשות return בתוך קטע של catch, ובכלל לא לתת ל try catch להכתיב את ה- flow של התוכנית.
מה שקורה בקוד זה שאני בודק האם משהו נמצא במנגון cache שלי, במידה ולא - אני קורא אותו מהכונן הקשיח ועושה השמה ב cache.
הסיבה שאני משתמש ב try ו- catch זה בגלל פעולת הקריאה מהכונן הקשיח.
איך הייתם משנים את הקוד? או שאולי הוא כן בסדר לפי עקרונות best practice?
תודה אלון

שתף

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

האשכול	מחבר	תאריך כתיבה	מספר
מבחינת best practice לא נהוג לתפוס exception כללי	Mr Andersson	15.05.19 10:14	1
תודה על התגובה. אם תאורתית יש סיכוי שווה שיחזור לי אחד מ3 סוגי אקספשן?	כמה עוד	15.05.19 10:28	2
אתה יכול לתפוס את שלושתם בשורה אחת או לתפוס כל אחד בנפרד	Mr Andersson	15.05.19 10:49	3
ואיך אתה יודע את זה לפי ההודעת שגיאה שאתה מחזיר?	Zeet3x	15.05.19 22:00	4
לפי סוג האקספשן שנזרק לא לפי הודעת השגיאה	Simple	15.06.19 15:48	6
כמו שרשמו לך על האקספשנים ובנוסף אני הייתי משתמש בטמפלייט	Simple	15.06.19 15:46	5
תודה!	כמה עוד	15.06.19 16:03	7

Mr Andersson
חבר מתאריך 19.3.02
6234 הודעות, 1 פידבק

10:14 15.05.19

1. מבחינת best practice לא נהוג לתפוס exception כללי
בתגובה להודעה מספר 0

אתה יודע מה אתה עושה בקוד, ולכן אתה יודע לאיזה שגיאה לצפות, אז עדיף לתפוס את הexception הספציפי שייזרק

http://i60.tinypic.com/35aowg8.jpg

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

כמה עוד

חבר מתאריך 18.11.09
234 הודעות, 11 פידבק

10:28 15.05.19

2. תודה על התגובה. אם תאורתית יש סיכוי שווה שיחזור לי אחד מ3 סוגי אקספשן?
בתגובה להודעה מספר 1

נגיד אחת מאלה יכולה לקפוץ לי:
בעיה בנתיב של הקובץ
בעיה עם הטקסט בתוך הקובץ
בעיה עם הקידוד (UTF או משהו כזה בקובץ)
?

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

Mr Andersson
חבר מתאריך 19.3.02
6234 הודעות, 1 פידבק

10:49 15.05.19

3. אתה יכול לתפוס את שלושתם בשורה אחת או לתפוס כל אחד בנפרד
בתגובה להודעה מספר 2

אם הטיפול הוא זהה בכל השגיאות אז תתפוס את שלושתם בשורה אחת

חוץ מזה הקוד נראה בסדר גמור לדעתי

http://i60.tinypic.com/35aowg8.jpg

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

Zeet3x

חבר מתאריך 14.2.12
9976 הודעות, 24 פידבק

22:00 15.05.19

4. ואיך אתה יודע את זה לפי ההודעת שגיאה שאתה מחזיר?
בתגובה להודעה מספר 2

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

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

Simple

חבר מתאריך 28.1.06
18765 הודעות, 22 פידבק

15:48 15.06.19

6. לפי סוג האקספשן שנזרק לא לפי הודעת השגיאה
בתגובה להודעה מספר 4

אם הבנתי אותך נכון

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

Simple

חבר מתאריך 28.1.06
18765 הודעות, 22 פידבק

15:46 15.06.19

5. כמו שרשמו לך על האקספשנים ובנוסף אני הייתי משתמש בטמפלייט
בתגובה להודעה מספר 0

ערכתי לאחרונה בתאריך 15.06.19 בשעה 15:55 בברכה, Simple

עבור הקריאה של הקובץ csv.
אני לא מתכנת בפייתון אז לא בטוח לגבי הסינטקס ומתעצל לבדוק אבל נניח ככה:
{1}/{0}.csv

זה קצת יותר קריא בעיניי.
בנוסף הייתי עושה קובץ של כל הסטרינגים בפרוייקט ושם בתוכו את הmessage שלך-
'cant...' במקום לרשום ככה ומשתמש בkey שנתת לסטרינג בקובץ ה סטרינגים.
הסיבה לכך היא שזה הרבה יותר קל לתחזוק ול reuse
אגב אם יש לך localization של הסטרינג לשפות אחרות קובץ הסטרינגים יעשה לך עבודה קלה
@כמה עוד@

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד

כמה עוד

חבר מתאריך 18.11.09
234 הודעות, 11 פידבק

16:03 15.06.19

7. תודה!
בתגובה להודעה מספר 0

(ניהול: מחק תגובה)

מכתב זה והנלווה אליו, על אחריות ועל דעת הכותב בלבד