שיחת משתמש:1Or/Vote.js

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

הערות:

  1. חושף ללא צורך פונקציות פרטיות לסקופ הגלובלי. אין שום צורך בכך: אפשר בקלות לקנן את הפונקציות הללו בתוך הפונקציה האנונימית שנקראת מ- then
  2. השתמשת בעוגנים, לצורך זיהוי הסעיפים בהם מתנהלת ההצבעה. זה לא אופטימלי. עדיף לבנות תבנית אחת זהה שמכניסים מיד אחרי ה-header, וללמד את הקוד שלך לחפור ב-DOM ולהוציא את כל המידע שהוא צריך. הקוד כמו שהוא עכשיו סובל מכך שהוא מאד שביר: למשל אם מישהו יערוך את הדף ויוסיף סעיף לפני הסעיפים הללו, הקוד יכשל, או גרוע מזה - יעשה משהו לא נכון.
  3. לא מספיק להודיע למשתמש שהצבעתו נרשמה - הוא מצפה לראותה בדף. הדרך הפשוטה לממש זאת היא על ידי טעינת הדף מחדש ( קריאה ל-windows.location.reload() )
  4. האופן בו אתה בונה את הקופסאות להצבעה בעייתי. בוודאי שכשעובדים עם jquery, לא מעבירים את הפונקציה של onclick כמחרוזת, אבל גם השאר בעייתי.
  5. לפרמטרים שהפונקציות מקבלות יש שמות סתומים ומבלבלים ( user, num, O ). האם user הוא המשתמש שמצביע, או זה עבורו מצביעים? מה זה num ? מה זה O ? (על הדרך: זה רעיון גרוע לקרוא למשתנה בשם O - קל מדי לבלבל זאת עם 0. יש גופנים בהם קל יותר להבדיל, ואחרים בהם זה כמעט בלתי אפשרי. גם בגופנים בהם ההבדל ניכר, קל מדי להתבלבל. ההערה הזו תקפה לאו דווקא ל-javascript, ושרירה לכל שפת תכנות בה כותבים)
  6. בצורה בה בנית את הסקריפט, הוא שימושי רק עבור הצבעה מסוימת, משום שהעוגנים שלך הם hardcoded בקוד של הסקריפט. זה לא נחוץ. מה שאתה רוצה זה למצוא עותקים של התבנית שלך (נניח בעזרת class - אי אפשר להשתמש ב-id משום שאסור לשים כמה אלמנטים עם אותו id), לחפש קצת למעלה ולמצוא את ה-id של הסעיף, ואז לפשפש ב-TOC ולמצוא את מספר הסעיף שאפשר להשתמש בו לצורך קריאה ל-API.
  7. כמובן, את כל הבניה של "בעד/נגד/בעד עם כוס תה/נגד עם אופניים", צריך להשאיר לתבנית - הסקריפט צריך למצוא את התבנית, להפוך את האלמנטים ל(המוסתרים) שהתבנית שמה לנראים, ולהדביק onclick מתאים לכל קופסה.

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

$('#' + O).append('<button style="border: 1px solid darkgray; border-radius: 3px; background: url(&quot;//upload.wikimedia.org/wikipedia/commons/7/79/Button_shade.png&quot;) black; padding: 2px; margin-bottom: 5px; width: 200px; font-size: 2.5em; color: #009900; cursor: pointer;" onclick="VoteToTheNextB(' + user + "," + num + "," + "'בעד'" + ')">בעד</button>');

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

$( '<button>' )
    .css( {
        border: '1px solid darkgrey'
        // and so on, add radius, background, padding, whatever, as independent fields
    } )
    .data( { item1: value1, item2: value2 } )
    .onclick ( yourfuntion )
    appendTo( parentElement ); // this is your " '#' + O
// same yourfunction can be used for all elements; inside yourfunction you can query the data items, like so:
function yourfunction() {
    var $element = $( this ); // this is the element that was clicked
    var valeu1 =$element.data( 'item1' );
}

אם תרצה לשפר/לשנות, ותרצה code review על הקוד אחרי השיפור, אשתדל לעזור. תוכל לבקש CR גם מאנשים אחרים - כל עוד אתה מקבל בהבנה כשלמישהו אין זמן או חשק. קיפודנחש 21:21, 13 בפברואר 2017 (IST)תגובה