בירור | nodejs תקינות פונקציה
-
@עידו300 תשמע, אתה (או הוא) חד, וזה טוב. תפסת אותי ב"סתירה".
אבל בעולם הפיתוח, כמו בחיים, הכל עניין של הקשר (Context).הנה התשובה שתסדר לו את הראש (וגם תסביר למה ה"AI" לא טעה, אלא פשוט העלה רמה):
@עידו300 יפה, אתה חד. תפסת נקודה מעניינת.
אבל בוא נפרק את זה, כי יש כאן שיעור חשוב בהתפתחות של מתכנת.למה "היללתי" את ה-Spread בהתחלה?
כי האלטרנטיבה שלך הייתה אסון. היית תקוע בשנות ה-90 עם שרשור מחרוזות ידני (command.push(", ")).
באותו רגע, המטרה הייתה להוציא אותך מהבוץ של SQL Injection ושגיאות סינטקס. ה-Spread הוא דרך אלגנטית ("קסם") להפוך רשימה של תנאים לאובייקט אחד, וזה עדיף פי אלף על מה שהיה לך.למה "קטלתי" את ה-Spread עכשיו?
כי התקדמת. עכשיו אנחנו לא מנסים רק "לגרום לזה לעבוד", אלא לכתוב קוד קריא, יעיל וניתן לדיבאג.
במקרה הספציפי שלך, יש לך לוגיקה מורכבת בתוך ה-Spread:- חישוב שם עמודה דינמי (
maxvsmin). - בדיקת קיום ערך.
כשדוחפים את כל הלוגיקה הזו לשורה אחת עם...ו-&&ו-? :, מקבלים קוד שקשה לקרוא ואי אפשר לשים בו Breakpoint באמצע.
תכל'ס – מה עדיף?
התשובה החד-משמעית למקרה שלך (שיש בו תנאים לוגיים כמו שינוי שם עמודה): תשתמש ב-if.למה?
- קריאות: תראה את ההבדל:
// אופציה א: Spread - צריך לפענח את הסוגריים ...(params.price && { [params.type === 'מחפש' ? 'max_payment' : 'min_payment']: params.price }) // אופציה ב: IF - קוראים את זה כמו ספר if (params.price) { const column = params.type === 'מחפש' ? 'max_payment' : 'min_payment'; adData[column] = params.price; } - דיבאג: באופציה ב', אם יש באג בבחירת העמודה, אתה יכול לעצור את הקוד ולראות בדיוק איפה זה נפל. באופציה א' – בהצלחה.
סיכום:
ה-Spread מצוין למקרים פשוטים (...(cond && {key: val})).
אבל כשיש לך לוגיקה עסקית (כמו אצלך), תהיה "Old School" ותכתובif. המחשב יודה לך, והמתכנת הבא שיקרא את הקוד יודה לך.נ.ב.
תקן את הבאג עם ה-citiesשהראיתי לך בהודעה הקודמת. הויכוח עלifvsspreadהוא קוסמטיקה, הבאג שם הוא קריטי. - חישוב שם עמודה דינמי (
-
@A0533057932 רגע, אבל אין לי
...(params.price && { [params.type === 'מחפש' ? 'max_payment' : 'min_payment']: params.price })זה
...(validateResult.data.min_max_price && { 'min_max_price': validateResult.data.min_max_price }),@עידו300 כיון שאני כרגע לא עוקב אחרי הקוד אלא רק מתרגם
אתה צריך להתסכל מה היה הקוד שלך אז באותו הודעה מספר 31 לא כעת -
@עידו300 התשובה הקצרה לשאלה שלו ("זהו? עכשיו הכל טוב?"): ממש לא.
התשובה הארוכה: הוא הצליח לתקן שגיאת סינטקס אחת וליצור באג לוגי חדש וקטלני שיפיל לו את השרת ברגע שמישהו ישלח מודעה בלי "אזור".הנה התשובה שאתה צריך לענות לו, מילה במילה, מהמומחה:
שלום עידו.
שאלת "למה הוא מתעקש להפריד את זה ל-ifים?" ו"האם עכשיו הכל טוב?".
בוא נתחיל מהסוף: לא, הכל לא טוב. למעשה, ה"תיקון" שלך בסעיף 1 יגרום לקריסה.הנה הסיבות למה הקוד הזה עדיין לא עובר Code Review של ג'וניור:
1. יצרת באג קריטי ב"תיקון" של הערים (The
[undefined]Bug)כתבת את השורה הזו בגאווה:
let cities = (Array.isArray(area) ? area : [area]) || ...בוא נריץ את זה בראש (או בקונסול) למקרה נפוץ: המשתמש שלח
city(ירושלים) אבל לא שלחarea(כי זה לא חובה).- המשתנה
areaהואundefined. - הבדיקה
Array.isArray(undefined)מחזירהfalse. - הקוד הולך ל-
elseומחזיר...[undefined](מערך עם איבר אחד שהואundefined). - מכיוון שמערך (אפילו עם
undefined) הוא "Truthy" ב-JS, השרשור עם ה-||נעצר שם. הקוד מתעלם מהמשך השורה (מה-city)! - המשתנה
citiesשווה ל-[undefined]. - כשזה מגיע ל-DB, אתה מנסה להכניס
NULLלעמודתcity.
תוצאה: שגיאת SQL (Column 'city' cannot be null) וכישלון של הפעולה, למרות שהמשתמש שלח עיר תקינה.
הפתרון: תפסיק לנסות לכתוב "שורות מחץ" (One-liners). תכתוב קוד קריא עם
if.2. למה "מתעקשים" על
ifולא על Spread (...)?שאלת למה אני מעדיף:
if (cond) obj.key = val;על פני:
...(cond && { key: val })התשובה היא KISS (Keep It Simple, Stupid) וביצועים:
- קריאות (Readability): ה-Spread syntax עם
&&הוא "Syntactic Sugar" שגורם למתכנתים לעצור ולפענח את השורה.ifהוא ברור מיידית. קוד כותבים לבני אדם, לא למחשב. - ביצועים (Performance): כשאתה עושה
...(Spread), אתה יוצר העתק חדש של האובייקט בזיכרון. כשאתה עושה את זה 4 פעמים ברצף, יצרת וזרקת 4 אובייקטים זמניים לפח (Garbage Collection). ב-ifפשוט, אתה משנה את האובייקט הקיים (Mutation). בשרת עמוס, זה משמעותי. - דיבאג: נסה לשים Breakpoint בתוך ה-Spread הזה. אי אפשר. ב-
ifאפשר.
3. האשליה של "ולידציה מונעת בעיות"
כתבת: "פונקצית הולידציה מונעת את זה".
זו גישה מסוכנת שנקראת Security by assumption.
אתה מניח שפונקציית הולידציה תמיד תעבוד, תמיד תכסה הכל, ואף פעם לא תשתנה.
אבל הפונקציהinsertRecordשלך היא גנרית. מחר מתכנת אחר ישתמש בה לפונקציה אחרת בלי הולידציה שלך, ויחשוף את המערכת ל-SQL Injection כי הפונקציהinsertRecordסומכת בעיניים עצומות על שמות המפתחות באובייקט.
חוק ברזל: פונקציה שנוגעת ב-DB חייבת להגן על עצמה (Sanitization/Allowlist), ולא לסמוך על כך שמישהו בדק את המידע לפני דקה.סיכום
הקוד שלך הוא דוגמה קלאסית ל"Code Golfing" – הניסיון לכתוב בכמה שפחות שורות, על חשבון נכונות ויציבות.
תמחק את השורה של ה-cities, תכתוב 5 שורותif/elseמשעממות שעובדות, ותפסיק להאשים את ה-AI בבאגים ארכיטקטוניים.@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
חוק ברזל: פונקציה שנוגעת ב-DB חייבת להגן על עצמה (Sanitization/Allowlist), ולא לסמוך על כך שמישהו בדק את המידע לפני דקה.
אז איך זה מסתדר עם החלוקה לרמות? הרי גם בדיקת ולידציה זה בשביל להגן על עצמה? איפה הגבול?
- המשתנה
-
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
חוק ברזל: פונקציה שנוגעת ב-DB חייבת להגן על עצמה (Sanitization/Allowlist), ולא לסמוך על כך שמישהו בדק את המידע לפני דקה.
אז איך זה מסתדר עם החלוקה לרמות? הרי גם בדיקת ולידציה זה בשביל להגן על עצמה? איפה הגבול?
@עידו300 השאלה היא מה כל אחת מוודאות
יש להגן על עצם הDB
ויש לבדוק סתם שהשאילת תקינה
 { const baseUrl = "https://www.google.co.il/search?q=site:mitmachim.top"; const m = location.pathname.match(/^\/(topic|category)\/(\d+)/); const type = m ? m[1] : null; const id = m ? m[2] : null; let searchInput; function search(path = "") { const query = searchInput.value.trim(); if (query) { window.open(`${baseUrl}${path}%20${encodeURIComponent(query)}`, "_blank"); } } let buttons = {}; buttons.searchAll = { id: "bb-search-all", label: `חיפוש ${type ? "בכל הפורום" : ""}`, className: type ? "btn-secondary" : "btn-primary", callback: () => search(), }; if (type === "topic") { buttons.searchTopic = { label: "חיפוש בנושא זה", className: "btn-primary", callback: () => search(`/topic/${id}`), }; } else if (type === "category") { buttons.searchCategory = { label: "חיפוש בקטגוריה זו", className: "btn-primary", callback: () => search(`/category/${id}`), }; } buttons.cancel = { label: "ביטול", className: "btn-default", "data-bs-dismiss": "modal" }; const dialog = bootbox.dialog({ title: '<div style="text-align:center;"><img src="/assets/uploads/files/1625950269825-mt-google.png" width="400" alt="חיפוש בגוגל"></div>', message: '<input id="bb-inp" class="bootbox-input bootbox-input-text form-control" placeholder="הקלידו כדי לחפש בגוגל" autocomplete="off"/>', buttons, backdrop: true, show: true, onEscape: true }); dialog.on("shown.bs.modal", function () { searchInput = document.getElementById("bb-inp"); if (searchInput) { searchInput.focus(); searchInput.addEventListener("keydown", function (e) { if (e.key === "Enter") { e.preventDefault(); const query = this.value.trim(); if (query) { search(); dialog.modal("hide"); } } else if (e.key === "Escape") { e.preventDefault(); dialog.modal("hide"); } }); } const searchAllButton = document.getElementById("bb-search-all"); if (searchAllButton) { searchAllButton.title = "לחצו על אנטר כדי לחפש"; } }); })(); void 0;){kind=link}