בירור | nodejs תקינות פונקציה
-
@עידו300 ואם תרצה בשפה בוטה יותר את הבעיות
ביקשת להחמיר? קיבלת.
אם החבר הזה היה מגיש את הקוד הזה ב-Code Review בחברה רצינית, הוא היה נשלח חזרה לשולחן עם הערה אחת: Delete and Rewrite.שאלת על Uncle Bob (רוברט מרטין) וספר ה-Clean Code שלו. התשובה היא חד משמעית: כן. חוקי הנדסת תוכנה לא נעלמים רק בגלל שאתה כותב ב-JavaScript. להיפך, בגלל ש-JS היא שפה דינמית וגמישה, המשמעת העצמית צריכה להיות הרבה יותר גבוהה כדי לא לייצר זבל.
הקוד הזה מפר כמעט כל עיקרון קדוש בספר. הנה "כתב האישום" לפי הסטנדרטים של Clean Code:
1. הפרה בוטה של SRP (עקרון האחריות היחידה)
בוב אומר: "לפונקציה צריכה להיות סיבה אחת בלבד להשתנות".
הפונקציה הזו היא מפלצת דו-ראשית (או חמש-ראשית). היא אחראית על:- ולידציה של קלט (Input Validation).
- בניית מחרוזות SQL (Query Building).
- לוגיקה עסקית (כללי "מחפש" מול "מפרסם").
- ניהול תשתית (פתיחת/סגירת חיבורים).
- זרימת מידע לטבלאות משנה (City).
למה זה נורא? אם מחר משנים את שם השדה ב-DB, אתה נוגע בפונקציה. אם משנים את חוקי הולידציה של הטלפון, אתה נוגע בפונקציה. אם מחליפים ספרית DB, אתה נוגע בפונקציה.
התוצאה: High Coupling. נגיעה קטנה במקום אחד תשבור משהו במקום אחר.2. הפרה של OCP (עקרון הפתיחות/סגירות)
הקוד צריך להיות "פתוח להרחבה, סגור לשינויים".
בקוד הזה, אם רוצים להוסיף שדה חדש (למשל "email"), אני חייב להיכנס לקרביים של הפונקציה, להוסיף עודif, עודpush, ולסכן את כל הלוגיקה הקיימת.
קוד נקי: היה מקבל אובייקט קונפיגורציה או משתמש ב-Mapper דינמי, כך שהוספת שדה לא דורשת שינוי לוגי בקוד הליבה.3. גיהנום של IF-ים (Arrow Code Anti-Pattern)
תסתכל על הצורה של הקוד. הוא נראה כמו חץ (
>).if (...) { if (...) { // ... } }זה נקרא Cyclomatic Complexity גבוה. רמת הקינון (Nesting) כאן היא אסון קוגניטיבי. המתכנת צריך "לזכור" באיזה עומק הוא נמצא ומה היו התנאים הקודמים.
החוק של בוב: אם יש לךifבתוךif- תוציא את זה לפונקציה נפרדת. הקוד צריך להיות שטוח ("Flatten your code").4. ערבוב רמות הפשטה (Mixing Levels of Abstraction)
זה אחד החטאים הגדולים.
באותה פונקציה יש לך לוגיקה עסקית גבוהה (checkProfession) ובו זמנית שרשור מחרוזות נמוך ומלוכלך (command.push(", profession")).
זה כמו ששף יחתוך את הבצל וגם יתקן את הצנרת של הכיור באותו הזמן.
את בניית השאילתה צריך להחביא מאחורי DAL (Data Access Layer) או פונקציית עזר. הקוד העסקי לא אמור לדעת איך כותביםINSERTב-SQL.5. שמות משתנים של חובבנים
results?results1?
באמת?
Uncle Bob אומר ששמות משתנים צריכים לגלות את כוונת המשורר (Intention Revealing).results1לא אומר לי כלום. תקרא לזהcityInsertResult.commandזה שם גרוע. זה לא פקודה, אלו שמות העמודות. תקרא לזהcolumns.- השימוש ב-Magic Strings (כמו
"all"או"מחפש") מפוזר בקוד במקום להשתמש ב-Constants או Enums.
6. חזרה על קוד (DRY - Don't Repeat Yourself)
יש כאן תבנית שחוזרת על עצמה 5 פעמים:
if (params.X && check(params.X)) { push(column); push(placeholder); push(value); } else { return error }זה שכפול קוד (Copy-Paste Programming). אם תרצה לשנות את הדרך שבה בונים שאילתה, תצטרך לתקן ב-5 מקומות שונים.
מתכנת JS רציני היה כותב פונקציית עזר גנרית שמקבלתfield,validator, ו-columnNameומריצה את הלוגיקה הזו פעם אחת.השורה התחתונה:
הקוד הזה הוא Procedural Spaghetti (ספגטי פרוצדורלי) שמנסה להתחפש לפונקציה מודרנית בגלל השימוש ב-
async/await.
הוא קשה לקריאה, קשה לתחזוקה, קשה לבדיקה (איך תכתוב Unit Test כשהכל דבוק ל-DB?), ובלתי אפשרי להרחבה.ב-JS אין קומפיילר שיצעק עליך, אבל יש את ה-Reviewer, והוא הרגע נתן לקוד הזה ציון נכשל.
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
- הפרה בוטה של SRP (עקרון האחריות היחידה)
בוב אומר: "לפונקציה צריכה להיות סיבה אחת בלבד להשתנות".
הפונקציה הזו היא מפלצת דו-ראשית (או חמש-ראשית). היא אחראית על:
ולידציה של קלט (Input Validation).
בניית מחרוזות SQL (Query Building).
לוגיקה עסקית (כללי "מחפש" מול "מפרסם").
ניהול תשתית (פתיחת/סגירת חיבורים).
זרימת מידע לטבלאות משנה (City)אני לא מבין איך זה אמור להיראות.
הרי אני לא יכול לזרוק פשוט את הפרמטרים למסד נתונים, צריך לבדוק שהם קיימים והם נכונים, הפונקציה בודקת רק שהם קימיים וקוראת לפונקציות משנה שבודקות את התקינות שלהם.ברור שצריך "לוגיקה עסקית" אחרת איך הפונקציה תדע להכניס רק מה שנכון להכניס? (אני לא סומך על המשתמש) אני לא חושב שנכון לעשות פוקציה למפרסם ופונקציה למחפש.
זרימת מידע וכו', בעצם כל קוד בnodejs הוא פונקציה שקוראת לפונקציות ומשמשת לכמה וכמה דברים, איך זה הולך עם הטענות כאן, ולצה זה לא נכון כלפי הזרימה שאצלי?
-
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
- הפרה בוטה של SRP (עקרון האחריות היחידה)
בוב אומר: "לפונקציה צריכה להיות סיבה אחת בלבד להשתנות".
הפונקציה הזו היא מפלצת דו-ראשית (או חמש-ראשית). היא אחראית על:
ולידציה של קלט (Input Validation).
בניית מחרוזות SQL (Query Building).
לוגיקה עסקית (כללי "מחפש" מול "מפרסם").
ניהול תשתית (פתיחת/סגירת חיבורים).
זרימת מידע לטבלאות משנה (City)אני לא מבין איך זה אמור להיראות.
הרי אני לא יכול לזרוק פשוט את הפרמטרים למסד נתונים, צריך לבדוק שהם קיימים והם נכונים, הפונקציה בודקת רק שהם קימיים וקוראת לפונקציות משנה שבודקות את התקינות שלהם.ברור שצריך "לוגיקה עסקית" אחרת איך הפונקציה תדע להכניס רק מה שנכון להכניס? (אני לא סומך על המשתמש) אני לא חושב שנכון לעשות פוקציה למפרסם ופונקציה למחפש.
זרימת מידע וכו', בעצם כל קוד בnodejs הוא פונקציה שקוראת לפונקציות ומשמשת לכמה וכמה דברים, איך זה הולך עם הטענות כאן, ולצה זה לא נכון כלפי הזרימה שאצלי?
@עידו300 אני חושד שאנחנו נכנסים פה לענינים אחרים
לכן אני אוודא רק
האם למדת בעבר שפת תכנות בצורה מסודרת?
כללי כתיבה?
OOP? SOLID? משהו מזה?
אני מתנצל מראה על הנימה אבל משהו בתשובה שלך מריח לי שיתכן והתשובה לדברים אלו היא לא - הפרה בוטה של SRP (עקרון האחריות היחידה)
-
@עידו300 אני חושד שאנחנו נכנסים פה לענינים אחרים
לכן אני אוודא רק
האם למדת בעבר שפת תכנות בצורה מסודרת?
כללי כתיבה?
OOP? SOLID? משהו מזה?
אני מתנצל מראה על הנימה אבל משהו בתשובה שלך מריח לי שיתכן והתשובה לדברים אלו היא לא@A0533057932 C# דרך ספר מסודר של מייקרוסופט. מדריך פה ושם קצת ספרים של האונ' הפתוחה, ל אמעבר
-
@A0533057932 C# דרך ספר מסודר של מייקרוסופט. מדריך פה ושם קצת ספרים של האונ' הפתוחה, ל אמעבר
@עידו300 https://github.com/jnguyen095/clean-code/blob/master/Clean.Code.A.Handbook.of.Agile.Software.Craftsmanship.pdf
ממליץ על הספר
אמנם הוא באנגלית
אולם היא פשוטה
וניתן גם להעתיק פרק פרק ולשלוח לבניה לתרגום
זה לגבי סגנון תכנות -
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
- הפרה בוטה של SRP (עקרון האחריות היחידה)
בוב אומר: "לפונקציה צריכה להיות סיבה אחת בלבד להשתנות".
הפונקציה הזו היא מפלצת דו-ראשית (או חמש-ראשית). היא אחראית על:
ולידציה של קלט (Input Validation).
בניית מחרוזות SQL (Query Building).
לוגיקה עסקית (כללי "מחפש" מול "מפרסם").
ניהול תשתית (פתיחת/סגירת חיבורים).
זרימת מידע לטבלאות משנה (City)אני לא מבין איך זה אמור להיראות.
הרי אני לא יכול לזרוק פשוט את הפרמטרים למסד נתונים, צריך לבדוק שהם קיימים והם נכונים, הפונקציה בודקת רק שהם קימיים וקוראת לפונקציות משנה שבודקות את התקינות שלהם.ברור שצריך "לוגיקה עסקית" אחרת איך הפונקציה תדע להכניס רק מה שנכון להכניס? (אני לא סומך על המשתמש) אני לא חושב שנכון לעשות פוקציה למפרסם ופונקציה למחפש.
זרימת מידע וכו', בעצם כל קוד בnodejs הוא פונקציה שקוראת לפונקציות ומשמשת לכמה וכמה דברים, איך זה הולך עם הטענות כאן, ולצה זה לא נכון כלפי הזרימה שאצלי?
@עידו300 וזה לגבי עצם הנקודה
וזה אני חושב גם דוד התכוון לומר לך
זאת שאלה מצוינת, והיא בדיוק הנקודה שמבדילה בין "מתכנת שכותב סקריפטים" לבין "ארכיטקט תוכנה".
אתה צודק ב-100% בדבר אחד: אסור לסמוך על המשתמש. חובה לבצע בדיקות תקינות (Validation).
אתה גם צודק שקוד ב-Node.js הוא רצף של קריאות לפונקציות.אבל, הטעות שלך היא ב-"איפה" ו-"איך" הדברים קורים.
כרגע הפונקציה שלך היא כמו מלצר במסעדה, שלא רק לוקח הזמנה, אלא גם רץ למטבח, שוחט את הפרה, מבשל את הסטייק, שוטף את הכלים וסולק את האשראי.הנה ההסבר למה הגישה שלך בעייתית ואיך עושים את זה נכון ("זרימת מידע" מקצועית):
הבעיה: ערבוב ה-"מה" עם ה-"איך"
בפונקציה שלך, הלוגיקה העסקית (מה מותר להכניס) וה-SQL (איך להכניס את זה למסד) שלובים זה בזה כמו צמה שלא ניתן להתיר.
- הלוגיקה העסקית: "אם זה 'מחפש', חובה שיהיה לו 'תדירות צינתוק'".
- התשתית (SQL): "צריך להוסיף פסיק ואז סימן שאלה לשאילתה".
ברגע ששמת את שניהם באותו
if, יצרת צימוד חזק (Tight Coupling).
הפונקציה שלך צריכה "לדעת" יותר מדי. היא צריכה לדעת ש-SQL דורש פסיקים בין שדות. ללוגיקה עסקית לא אמור להיות אכפת מפסיקים!הפתרון: הפרדת שכבות (Layers)
בפיתוח מקצועי, אנחנו מפרקים את התהליך ל-3 שלבים ברורים. כל שלב הוא פונקציה/מחלקה נפרדת:
שלב 1: ה-Validator (השוטר בכניסה)
הוא מקבל את ה-
params, בודק שהם תקינים, וזורק שגיאה אם לא. הוא לא יודע שיש מסד נתונים בכלל. הוא רק יודע מה זה "מספר טלפון תקין".שלב 2: ה-Service (המנהל העסקי)
הוא מקבל מידע נקי מה-Validator. הוא מפעיל את החוקים העסקיים (למשל: "אם זה מחפש, תוסיף את שדה הצינתוק לאובייקט"). הוא מכין אובייקט מידע מוכן לשמירה. הוא לא יודע איך כותבים SQL.
שלב 3: ה-Repository / DAL (הפועל השחור)
הוא מקבל אובייקט מוכן, וכל תפקידו הוא לתרגם אותו ל-SQL ולהכניס למסד. הוא לא שואל שאלות כמו "האם הטלפון תקין?" (כי זה נבדק בשלב 1) או "האם למחפש מותר שדה כזה?" (כי זה טופל בשלב 2). הוא פשוט שומר.
איך זה נראה בקוד (הוכחה שזה אפשרי ונקי)
תראה כמה זה קריא וקל לתחזוקה כשמפרידים:
1. ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js async function insertRecord(connection, tableName, dataObject) { const columns = Object.keys(dataObject); const values = Object.values(dataObject); const placeholders = columns.map(() => '?').join(', '); // בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values); }2. ה-Service (הפונקציה הראשית שלך - מתוקנת)
עכשיו הפונקציה שלך מתעסקת רק במה ולא באיך.
// adService.js import { validateAdInput } from './validators'; // קובץ נפרד לבדיקות import { insertRecord } from './dbUtils'; export async function addAd(params) { // 1. ולידציה - זורק שגיאה אם משהו לא תקין // הפונקציה הזו מכילה את כל ה-checkType, checkPhone שלך const cleanData = validateAdInput(params); // 2. הכנת המידע (לוגיקה עסקית) // בניית האובייקט בצורה נקייה const adToInsert = { type: cleanData.type, profession: cleanData.profession, phone: cleanData.phone, recording_path: cleanData.path }; // לוגיקה עסקית: טיפול במחיר לפי סוג if (cleanData.type === 'מחפש') { adToInsert.max_payment = cleanData.price; adToInsert.ring_mode = cleanData.ringMode; // הולדיציה כבר וידאה שזה קיים } else { adToInsert.min_payment = cleanData.price; // למפרסם אין ring_mode, אז פשוט לא מכניסים אותו לאובייקט } // 3. עבודה מול ה-DB (תשתית) const connection = await pool.getConnection(); try { await connection.beginTransaction(); // הכנסת המודעה - שים לב כמה זה נקי! בלי פסיקים ובלי שרשורים const [result] = await insertRecord(connection, 'ads', adToInsert); const adId = result.insertId; // טיפול בערים (לוגיקה עסקית נוספת) if (cleanData.cities && cleanData.cities.length > 0) { // כאן תהיה קריאה לפונקציית עזר להכנסת ערים (Bulk Insert) await insertCities(connection, adId, cleanData.cities); } await connection.commit(); return { success: true, id: adId }; } catch (e) { await connection.rollback(); throw e; } finally { connection.release(); } }למה זה יותר טוב? (תשובה לחבר שלך)
- בטיחות: אין סיכוי שתשכח פסיק בשאילתה ותפיל את השרת, כי ה-
insertRecordבונה את השאילתה אוטומטית. - קריאות: במבט אחד על
adToInsertאתה מבין בדיוק אלו שדות נכנסים ל-DB. בקוד המקורי צריך לפענח את ה-command.push. - שינויים עתידיים:
- רוצה להוסיף ולידציה? לך לקובץ
validators. הפונקציה הראשית לא משתנה. - רוצה להוסיף שדה? תוסיף אותו ל-
adToInsert. לא צריך לגעת ב-SQL. - רוצה לעבור ל-PostgreSQL? תחליף רק את
insertRecord. הלוגיקה העסקית נשארת זהה.
- רוצה להוסיף ולידציה? לך לקובץ
זה ההבדל בין קוד שעובד בטעות, לקוד שיעבוד גם בעוד שנתיים.
- הפרה בוטה של SRP (עקרון האחריות היחידה)
-
האם הפונקציה הזו תקינה? או שחסרים/מיותרים בה דברים?
gpt מביא רשימה שאני ממש לא מסכים איתה.
אשמח לשמוע את דעתכם.הפונקציה מוסיפה מודעה, מוודאת שיש את הפרמטרים החובה ושהם תקינים, מוסיפה פרמטרי רשות אם ישנם ותקינים.
לאחר פרסום מודעה, מוסיפה לטבלת ערים מקושרת את ID המודעה ואת הערים שאליהם היא שייכת.// פונקציה להוספת מודעה חדשה למסד הנתונים export async function addAd(params) { let command = []; let placeholders = []; let values = []; // מחפש/מפרסם - חובה if (params.type && checkType(params.type)) { command.push("type"); placeholders.push("?"); values.push(params.type); } else { return { error: "Invalid type" }; } // מקצוע - חובה if (params.profession && checkProfession(params.profession)) { command.push(", profession"); placeholders.push(", ?"); values.push(params.profession); } else { return { error: "Invalid profession" }; } // טלפון - חובה if (params.phone && isValidIsraeliPhone(params.phone)) { command.push(", phone"); placeholders.push(", ?"); values.push(params.phone); } else { return { error: "Invalid phone number" }; } // מינימום/מקסימום מחיר - לא חובה, אבל אם קיים חייב להיות מספר חיובי ושלם if (params.price) { if (checkNumber(params.price)) { command.push((params.type === "מחפש") ? ", max_payment" : ", min_payment"); placeholders.push(", ?"); values.push(params.price); } else { return { error: "Invalid price" }; } } // תדירות צינתוק - חובה רק במחפש, אסור שיהיה במפרסם if (params.type === "מחפש") { if (params.ringMode && checkRingMode(params.ringMode)) { command.push(", ring_mode"); placeholders.push(", ?"); values.push(params.ringMode); } else { return { error: "Invalid ring mode" }; } } if (params.path) { command.push(", recording_path"); placeholders.push(", ?"); values.push(params.path); } let connection; connection = await mysql.createConnection({ host, user, password, database }); await connection.beginTransaction(); const [results] = await connection.execute(`INSERT INTO ads (${command.join('')}) VALUES (${placeholders.join('')})`, values ); // אזור - לא חובה, אבל אם קיים חייב להיות תקין if (params.area) { if (checkArea(params.area)) { const [results1] = await connection.execute(`INSERT INTO ad_cities (ad_id, city) VALUES (?, ?)`, [results.insertId, params.area]); await connection.commit(); return results1; } else { return { error: "Invalid area" }; } } // עיר - לא חובה, אבל אם קיים חייב להיות תקין else if (params.city) { if (Array.isArray(params.city)) { for (const city of params.city) { if (checkCity(city)) { await connection.execute(`INSERT INTO ad_cities (ad_id, city) VALUES (?, ?)`, [results.insertId, city]); } else { return { error: `Invalid city: ${city}` }; } } await connection.commit(); return { success: true }; } } else {const [results1] = await connection.execute(`INSERT INTO ad_cities (ad_id, city) VALUES (?, ?)`, [results.insertId, "all"]); await connection.commit(); return results1; } }בפורום אחר פשוט טענו שאני לא יודע js (מה שדי נכון, למדתי תוך כדי עבודה, לא לעומק).
יש למישהו תשובה יותר טובה? או שיוכל להסביר לי למה זה כ"כ גרוע בJS?עריכה: עשיתי שינויים קלים, לא משהו מהותי.
@עידו300 ולעצם העניין
תעצור ותלמד על שלשת השכבות
על SOLID (אמרת שלמדת C# אז אתה אמור להיבן את זה)
על קלין קוד
ותתחיל ממש לשלוח לAI פונקציה
ותאמר לו שכתב לי אותה מחדש לפי הכללים XZY ותתחיל לראות מה ההוא שינה
וכך תתחיל לאט לאט לחשוב בצורה הזאת
כמו שאמרת
לחשוב כמו מישהו מהתעשייה -
@עידו300 ולעצם העניין
תעצור ותלמד על שלשת השכבות
על SOLID (אמרת שלמדת C# אז אתה אמור להיבן את זה)
על קלין קוד
ותתחיל ממש לשלוח לAI פונקציה
ותאמר לו שכתב לי אותה מחדש לפי הכללים XZY ותתחיל לראות מה ההוא שינה
וכך תתחיל לאט לאט לחשוב בצורה הזאת
כמו שאמרת
לחשוב כמו מישהו מהתעשייה@A0533057932 וואו זה הרבה עבודה בשביל פרויקט כ"כ קטן.... בטוח שעושים ככה לפרוקייטים כאלה קטנים?
-
@A0533057932 וואו זה הרבה עבודה בשביל פרויקט כ"כ קטן.... בטוח שעושים ככה לפרוקייטים כאלה קטנים?
-
@עידו300 וזה לגבי עצם הנקודה
וזה אני חושב גם דוד התכוון לומר לך
זאת שאלה מצוינת, והיא בדיוק הנקודה שמבדילה בין "מתכנת שכותב סקריפטים" לבין "ארכיטקט תוכנה".
אתה צודק ב-100% בדבר אחד: אסור לסמוך על המשתמש. חובה לבצע בדיקות תקינות (Validation).
אתה גם צודק שקוד ב-Node.js הוא רצף של קריאות לפונקציות.אבל, הטעות שלך היא ב-"איפה" ו-"איך" הדברים קורים.
כרגע הפונקציה שלך היא כמו מלצר במסעדה, שלא רק לוקח הזמנה, אלא גם רץ למטבח, שוחט את הפרה, מבשל את הסטייק, שוטף את הכלים וסולק את האשראי.הנה ההסבר למה הגישה שלך בעייתית ואיך עושים את זה נכון ("זרימת מידע" מקצועית):
הבעיה: ערבוב ה-"מה" עם ה-"איך"
בפונקציה שלך, הלוגיקה העסקית (מה מותר להכניס) וה-SQL (איך להכניס את זה למסד) שלובים זה בזה כמו צמה שלא ניתן להתיר.
- הלוגיקה העסקית: "אם זה 'מחפש', חובה שיהיה לו 'תדירות צינתוק'".
- התשתית (SQL): "צריך להוסיף פסיק ואז סימן שאלה לשאילתה".
ברגע ששמת את שניהם באותו
if, יצרת צימוד חזק (Tight Coupling).
הפונקציה שלך צריכה "לדעת" יותר מדי. היא צריכה לדעת ש-SQL דורש פסיקים בין שדות. ללוגיקה עסקית לא אמור להיות אכפת מפסיקים!הפתרון: הפרדת שכבות (Layers)
בפיתוח מקצועי, אנחנו מפרקים את התהליך ל-3 שלבים ברורים. כל שלב הוא פונקציה/מחלקה נפרדת:
שלב 1: ה-Validator (השוטר בכניסה)
הוא מקבל את ה-
params, בודק שהם תקינים, וזורק שגיאה אם לא. הוא לא יודע שיש מסד נתונים בכלל. הוא רק יודע מה זה "מספר טלפון תקין".שלב 2: ה-Service (המנהל העסקי)
הוא מקבל מידע נקי מה-Validator. הוא מפעיל את החוקים העסקיים (למשל: "אם זה מחפש, תוסיף את שדה הצינתוק לאובייקט"). הוא מכין אובייקט מידע מוכן לשמירה. הוא לא יודע איך כותבים SQL.
שלב 3: ה-Repository / DAL (הפועל השחור)
הוא מקבל אובייקט מוכן, וכל תפקידו הוא לתרגם אותו ל-SQL ולהכניס למסד. הוא לא שואל שאלות כמו "האם הטלפון תקין?" (כי זה נבדק בשלב 1) או "האם למחפש מותר שדה כזה?" (כי זה טופל בשלב 2). הוא פשוט שומר.
איך זה נראה בקוד (הוכחה שזה אפשרי ונקי)
תראה כמה זה קריא וקל לתחזוקה כשמפרידים:
1. ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js async function insertRecord(connection, tableName, dataObject) { const columns = Object.keys(dataObject); const values = Object.values(dataObject); const placeholders = columns.map(() => '?').join(', '); // בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values); }2. ה-Service (הפונקציה הראשית שלך - מתוקנת)
עכשיו הפונקציה שלך מתעסקת רק במה ולא באיך.
// adService.js import { validateAdInput } from './validators'; // קובץ נפרד לבדיקות import { insertRecord } from './dbUtils'; export async function addAd(params) { // 1. ולידציה - זורק שגיאה אם משהו לא תקין // הפונקציה הזו מכילה את כל ה-checkType, checkPhone שלך const cleanData = validateAdInput(params); // 2. הכנת המידע (לוגיקה עסקית) // בניית האובייקט בצורה נקייה const adToInsert = { type: cleanData.type, profession: cleanData.profession, phone: cleanData.phone, recording_path: cleanData.path }; // לוגיקה עסקית: טיפול במחיר לפי סוג if (cleanData.type === 'מחפש') { adToInsert.max_payment = cleanData.price; adToInsert.ring_mode = cleanData.ringMode; // הולדיציה כבר וידאה שזה קיים } else { adToInsert.min_payment = cleanData.price; // למפרסם אין ring_mode, אז פשוט לא מכניסים אותו לאובייקט } // 3. עבודה מול ה-DB (תשתית) const connection = await pool.getConnection(); try { await connection.beginTransaction(); // הכנסת המודעה - שים לב כמה זה נקי! בלי פסיקים ובלי שרשורים const [result] = await insertRecord(connection, 'ads', adToInsert); const adId = result.insertId; // טיפול בערים (לוגיקה עסקית נוספת) if (cleanData.cities && cleanData.cities.length > 0) { // כאן תהיה קריאה לפונקציית עזר להכנסת ערים (Bulk Insert) await insertCities(connection, adId, cleanData.cities); } await connection.commit(); return { success: true, id: adId }; } catch (e) { await connection.rollback(); throw e; } finally { connection.release(); } }למה זה יותר טוב? (תשובה לחבר שלך)
- בטיחות: אין סיכוי שתשכח פסיק בשאילתה ותפיל את השרת, כי ה-
insertRecordבונה את השאילתה אוטומטית. - קריאות: במבט אחד על
adToInsertאתה מבין בדיוק אלו שדות נכנסים ל-DB. בקוד המקורי צריך לפענח את ה-command.push. - שינויים עתידיים:
- רוצה להוסיף ולידציה? לך לקובץ
validators. הפונקציה הראשית לא משתנה. - רוצה להוסיף שדה? תוסיף אותו ל-
adToInsert. לא צריך לגעת ב-SQL. - רוצה לעבור ל-PostgreSQL? תחליף רק את
insertRecord. הלוגיקה העסקית נשארת זהה.
- רוצה להוסיף ולידציה? לך לקובץ
זה ההבדל בין קוד שעובד בטעות, לקוד שיעבוד גם בעוד שנתיים.
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
- ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js
async function insertRecord(connection, tableName, dataObject) {
const columns = Object.keys(dataObject);
const values = Object.values(dataObject);
const placeholders = columns.map(() => '?').join(', ');// בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values);}
אני לא מבין, אם זה כ"כ בסיסי בעבודה עם מסד נתונים, וזו בעצם הפונקציה היחידה שצריך (להוספה) איך זה שבספריה של הsql אן פונקציה כזאת? זה לא בזבוז לכתוב כזו פונקציה כל פרוקיט מחדש? (שואל להבין לא בא להתנגח או משהו).
-
@עידו300 בטח!
תחשב את כל השעות שלך ושל כל אלו שמנסים לעזור לך כאן ובתחומים ותיצא לך חשבון יותר גדול מהשעות שיקח לך ללמוד JS בסיסי. -
@מד אוקיי, אז אני מבין ממך ומ@a0533057932 שבjs בסיסי אתם לא מתכוונים לסמנטיקה של השפה (אל תתפסו אותי על המילה הנכונה, הבנתם מה אני מתכוון), אלא לצורת חשיבה שלה?
לא יודע, לא מצליח... לא תופס את הנקודה
-
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
- ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js
async function insertRecord(connection, tableName, dataObject) {
const columns = Object.keys(dataObject);
const values = Object.values(dataObject);
const placeholders = columns.map(() => '?').join(', ');// בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values);}
אני לא מבין, אם זה כ"כ בסיסי בעבודה עם מסד נתונים, וזו בעצם הפונקציה היחידה שצריך (להוספה) איך זה שבספריה של הsql אן פונקציה כזאת? זה לא בזבוז לכתוב כזו פונקציה כל פרוקיט מחדש? (שואל להבין לא בא להתנגח או משהו).
@עידו300 עבור sql משתמשים במנגנון כזה לפי מה שאני מכיר
https://share.google/aimode/fgFy5KpjTkJlohftG
זה מה שידוע לי הבסט פרקטיסט
במיוחד בoop
גילוי נאות
לא מכיר את הספריות הנ״ל
אני משתמש בפייתון במנגנונים דומים - ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
-
@עידו300 וזה לגבי עצם הנקודה
וזה אני חושב גם דוד התכוון לומר לך
זאת שאלה מצוינת, והיא בדיוק הנקודה שמבדילה בין "מתכנת שכותב סקריפטים" לבין "ארכיטקט תוכנה".
אתה צודק ב-100% בדבר אחד: אסור לסמוך על המשתמש. חובה לבצע בדיקות תקינות (Validation).
אתה גם צודק שקוד ב-Node.js הוא רצף של קריאות לפונקציות.אבל, הטעות שלך היא ב-"איפה" ו-"איך" הדברים קורים.
כרגע הפונקציה שלך היא כמו מלצר במסעדה, שלא רק לוקח הזמנה, אלא גם רץ למטבח, שוחט את הפרה, מבשל את הסטייק, שוטף את הכלים וסולק את האשראי.הנה ההסבר למה הגישה שלך בעייתית ואיך עושים את זה נכון ("זרימת מידע" מקצועית):
הבעיה: ערבוב ה-"מה" עם ה-"איך"
בפונקציה שלך, הלוגיקה העסקית (מה מותר להכניס) וה-SQL (איך להכניס את זה למסד) שלובים זה בזה כמו צמה שלא ניתן להתיר.
- הלוגיקה העסקית: "אם זה 'מחפש', חובה שיהיה לו 'תדירות צינתוק'".
- התשתית (SQL): "צריך להוסיף פסיק ואז סימן שאלה לשאילתה".
ברגע ששמת את שניהם באותו
if, יצרת צימוד חזק (Tight Coupling).
הפונקציה שלך צריכה "לדעת" יותר מדי. היא צריכה לדעת ש-SQL דורש פסיקים בין שדות. ללוגיקה עסקית לא אמור להיות אכפת מפסיקים!הפתרון: הפרדת שכבות (Layers)
בפיתוח מקצועי, אנחנו מפרקים את התהליך ל-3 שלבים ברורים. כל שלב הוא פונקציה/מחלקה נפרדת:
שלב 1: ה-Validator (השוטר בכניסה)
הוא מקבל את ה-
params, בודק שהם תקינים, וזורק שגיאה אם לא. הוא לא יודע שיש מסד נתונים בכלל. הוא רק יודע מה זה "מספר טלפון תקין".שלב 2: ה-Service (המנהל העסקי)
הוא מקבל מידע נקי מה-Validator. הוא מפעיל את החוקים העסקיים (למשל: "אם זה מחפש, תוסיף את שדה הצינתוק לאובייקט"). הוא מכין אובייקט מידע מוכן לשמירה. הוא לא יודע איך כותבים SQL.
שלב 3: ה-Repository / DAL (הפועל השחור)
הוא מקבל אובייקט מוכן, וכל תפקידו הוא לתרגם אותו ל-SQL ולהכניס למסד. הוא לא שואל שאלות כמו "האם הטלפון תקין?" (כי זה נבדק בשלב 1) או "האם למחפש מותר שדה כזה?" (כי זה טופל בשלב 2). הוא פשוט שומר.
איך זה נראה בקוד (הוכחה שזה אפשרי ונקי)
תראה כמה זה קריא וקל לתחזוקה כשמפרידים:
1. ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js async function insertRecord(connection, tableName, dataObject) { const columns = Object.keys(dataObject); const values = Object.values(dataObject); const placeholders = columns.map(() => '?').join(', '); // בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values); }2. ה-Service (הפונקציה הראשית שלך - מתוקנת)
עכשיו הפונקציה שלך מתעסקת רק במה ולא באיך.
// adService.js import { validateAdInput } from './validators'; // קובץ נפרד לבדיקות import { insertRecord } from './dbUtils'; export async function addAd(params) { // 1. ולידציה - זורק שגיאה אם משהו לא תקין // הפונקציה הזו מכילה את כל ה-checkType, checkPhone שלך const cleanData = validateAdInput(params); // 2. הכנת המידע (לוגיקה עסקית) // בניית האובייקט בצורה נקייה const adToInsert = { type: cleanData.type, profession: cleanData.profession, phone: cleanData.phone, recording_path: cleanData.path }; // לוגיקה עסקית: טיפול במחיר לפי סוג if (cleanData.type === 'מחפש') { adToInsert.max_payment = cleanData.price; adToInsert.ring_mode = cleanData.ringMode; // הולדיציה כבר וידאה שזה קיים } else { adToInsert.min_payment = cleanData.price; // למפרסם אין ring_mode, אז פשוט לא מכניסים אותו לאובייקט } // 3. עבודה מול ה-DB (תשתית) const connection = await pool.getConnection(); try { await connection.beginTransaction(); // הכנסת המודעה - שים לב כמה זה נקי! בלי פסיקים ובלי שרשורים const [result] = await insertRecord(connection, 'ads', adToInsert); const adId = result.insertId; // טיפול בערים (לוגיקה עסקית נוספת) if (cleanData.cities && cleanData.cities.length > 0) { // כאן תהיה קריאה לפונקציית עזר להכנסת ערים (Bulk Insert) await insertCities(connection, adId, cleanData.cities); } await connection.commit(); return { success: true, id: adId }; } catch (e) { await connection.rollback(); throw e; } finally { connection.release(); } }למה זה יותר טוב? (תשובה לחבר שלך)
- בטיחות: אין סיכוי שתשכח פסיק בשאילתה ותפיל את השרת, כי ה-
insertRecordבונה את השאילתה אוטומטית. - קריאות: במבט אחד על
adToInsertאתה מבין בדיוק אלו שדות נכנסים ל-DB. בקוד המקורי צריך לפענח את ה-command.push. - שינויים עתידיים:
- רוצה להוסיף ולידציה? לך לקובץ
validators. הפונקציה הראשית לא משתנה. - רוצה להוסיף שדה? תוסיף אותו ל-
adToInsert. לא צריך לגעת ב-SQL. - רוצה לעבור ל-PostgreSQL? תחליף רק את
insertRecord. הלוגיקה העסקית נשארת זהה.
- רוצה להוסיף ולידציה? לך לקובץ
זה ההבדל בין קוד שעובד בטעות, לקוד שיעבוד גם בעוד שנתיים.
@A0533057932 כתב בבירור | nodejs תקינות פונקציה:
איך זה נראה בקוד (הוכחה שזה אפשרי ונקי)
תראה כמה זה קריא וקל לתחזוקה כשמפרידים:- ה-Repository (מטפל רק ב-SQL, בלי לוגיקה)
פונקציה גנרית שיודעת לקבל אובייקט ולדחוף ל-DB. כתבת אותה פעם אחת, והיא עובדת לכל הטבלאות.
// dbUtils.js
async function insertRecord(connection, tableName, dataObject) {
const columns = Object.keys(dataObject);
const values = Object.values(dataObject);
const placeholders = columns.map(() => '?').join(', ');// בונה SQL באופן דינמי - אין יותר שרשור מחרוזות ידני עם פסיקים! const sql = `INSERT INTO ${tableName} (${columns.join(', ')}) VALUES (${placeholders})`; return connection.execute(sql, values);}
2. ה-Service (הפונקציה הראשית שלך - מתוקנת)
עכשיו הפונקציה שלך מתעסקת רק במה ולא באיך.// adService.js
import { validateAdInput } from './validators'; // קובץ נפרד לבדיקות
import { insertRecord } from './dbUtils';export async function addAd(params) {
// 1. ולידציה - זורק שגיאה אם משהו לא תקין
// הפונקציה הזו מכילה את כל ה-checkType, checkPhone שלך
const cleanData = validateAdInput(params);// 2. הכנת המידע (לוגיקה עסקית) // בניית האובייקט בצורה נקייה const adToInsert = { type: cleanData.type, profession: cleanData.profession, phone: cleanData.phone, recording_path: cleanData.path }; // לוגיקה עסקית: טיפול במחיר לפי סוג if (cleanData.type === 'מחפש') { adToInsert.max_payment = cleanData.price; adToInsert.ring_mode = cleanData.ringMode; // הולדיציה כבר וידאה שזה קיים } else { adToInsert.min_payment = cleanData.price; // למפרסם אין ring_mode, אז פשוט לא מכניסים אותו לאובייקט } // 3. עבודה מול ה-DB (תשתית) const connection = await pool.getConnection(); try { await connection.beginTransaction(); // הכנסת המודעה - שים לב כמה זה נקי! בלי פסיקים ובלי שרשורים const [result] = await insertRecord(connection, 'ads', adToInsert); const adId = result.insertId; // טיפול בערים (לוגיקה עסקית נוספת) if (cleanData.cities && cleanData.cities.length > 0) { // כאן תהיה קריאה לפונקציית עזר להכנסת ערים (Bulk Insert) await insertCities(connection, adId, cleanData.cities); } await connection.commit(); return { success: true, id: adId }; } catch (e) { await connection.rollback(); throw e; } finally { connection.release(); }}
הAI מתעלם מזה שיש פרמטרים אופציונליים, הרי אם יש פרמטרים אופציונליים אני חייב לעבור לראות אם הם שם, זה כבר די מחזיר אותנו לקוד שלי...
נכון, הוא הוציא פונקציה גנרית להוספה למסד נתונים, וככל הנראה עוד פונקציה שתעבור בעצמה על כל הפרמטרים ותעשה להם ולידציה (אותה הוא לא כתב) אבל חוץ ממנה הוא השאיר הכל אותו הדבר. כולל המון שאילתות למסד נתונים.