כתיבה נכונה של פונקציית executor עבור פרומיס

[yossiz]

הנה שאלה עבור ראיון עבודה... עד אתמול לא נראה לי שהייתי מצליח לענות תשובה נכונה על השאלה, למדתי את התשובה מנסיון. תזכורת לעצמי: פעם הבא אל תתעלם מאזהרות eslint (ולמי שלא משתמש, אני ממליץ בחום להשתמש ב-standardjs - [תוסף ל-vscode]).

הנה קטע קוד:

function getFooPromise () {
  return new Promise(async (resolve, reject) => {
    const bar = await getBar();
    bar.getFooCallbackStyle((error, data) => {
      if (error) return reject(error);
      resolve(data);
    });
  });
}

שאלות:

• מה לא טוב בקוד הנ"ל?
• איך לתקן אותו?

(נסה לענות על השאלות בלי להסתכל בתשובה...)

תשובה

[חוקר]

לא הכי נכנסתי לסיפור ולא נכנסתי ללינק של התשובה
אבל אני הייתי כותב כך כזו פונקציה:

function getFooPromise () {
  return new Promise((resolve, reject) => {
    getBar().then(bar => {
      bar.getFooCallbackStyle((error, data) => {
        if (error) return reject(error);
        resolve(data);
      });
    });
  });
}

היות שבין כך הפרומיז יחזיר את הערך רק בסיום הביצוע לכן לא הייתי משתמש עם async/await

[אהרן]

@חוקר אמר בכתיבה נכונה של פונקציית executor עבור פרומיס:

בין כך הפרומיז יחזיר את הערך רק בסיום הביצוע

מה פתאום?!

[yossiz]

@חוקר הקוד שלך זהה לגמרי לקוד שלי. אני לא רואה שום הבדל ממשי רק תחבירי. הבעיה קיימת גם בקוד שלך.

@חוקר אמר בכתיבה נכונה של פונקציית executor עבור פרומיס:

היות שבין כך הפרומיז יחזיר את הערך רק בסיום הביצוע לכן לא הייתי משתמש עם async/await

לא הבנתי את הטיעון

[dovid]

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

הייתי כותב או ככה:

function getFooPromise () {
  return getBar().then(x => new Promise((resolve, reject) => {
    bar.getFooCallbackStyle((error, data) => {
      if (error) return reject(error);
      resolve(data);
    });
  });
}

או ככה:

async function getFooPromise () {
  const bar = await getBar();
  return new Promise(async (resolve, reject) => {
    bar.getFooCallbackStyle((error, data) => {
      if (error) return reject(error);
      resolve(data);
    });
  });
}

[yossiz]

הקדמה כללית לפרומיסים (למי שכבר מכיר איך פרומיס עובד יש פה הרבה אריכות מיותרת... אולי מי שעוד לא מכיר טוב יקבל מזה תועלת)
ב-JS - כמו בכל שפה אחרת - יש פעולות שלוקחות זמן, מה ששונה ב-JS: שאנחנו מאוד לא רוצים להפסיק את הכל ולחכות עד שזה יקרה (כי כידוע יש רק ט'רד אחד, ואם הט'רד מושבת - כל הדף מושבת). הפתרון לזה הוא להקים תור של פעולות, בצורה שסיום פעולה א' תגרום להרצת פעולה ב' וכן הלאה, המשימה שלנו הוא רק ל"הצית את ניצוץ הראשון" של הפעולה הראשונה, וכל השאר רץ מאיליו בלי התערבות שלנו.
בימי בראשית היו עושים את זה על ידי שהיו מעבירים לפעולה הראשונה את הפעולה השנייה, כך שהפעולה עצמה - שהיא יודעת מתי היא מסתיימת - היא תתחיל את הפעולה השניה, ולפעולה השנייה מעבירים את הפעולה השלישית, וכן הלאה. בעצם עשינו שרשרת של פעולות שכל טבעת בשרשרת מחזיקה את הפעולה הבאה.
הבעיה בזה מבחינה תחבירית הוא שצורה זו מחייבת הרבה קינון. אם אנחנו רוצים להקים שרשרת של שלוש פעולות, סדר הדברים הוא שנכתוב את הפעולה הראשונה, נעביר לה את הפעולה השנייה, ולשנייה נעביר את השלישית. כולנו מכירים את פירמידת הגיהנום שזה גורם.
אפשר לדמות את זה לשרשרת אמיתית שאם נרצה להוסיף עוד טבעת זה מחייב אותנו להתכופף לסוף השרשרת כדי להגיע לטבעת האחרונה כדי לתלות עליו עוד טבעות.
הפתרון לזה הגיעה בצורת פרומיסים. בצורה של פרומיס - במקום שנעביר את הפעולה הבאה לפעולה הראשונה - הפעולה הראשונה מחזירה לנו ידית. ידית זו מיוחדת, שהיא מגיבה להצלחתה או כשלונה של הפעולה. ואנחנו מדביקים את הפעולה השנייה לידית של הפעולה הראשונה בצורה שהצלחת הפעולה הראשונה תגרום לפעולה השנייה להתחיל (סביבת JS דואגת לקסם הזה לעבוד).
היתרון בזה הוא שאין צורך בקינון. יש לנו את הידית ביד ולא צריך להתכופף לתוך הפעולה כדי לקבל את המתלה שעליו אנו תולים את הפעולה הבאה.
הרעיון של בנאי ה-Promise הוא לקחת פעולה שבנויה לקבל callback שזו צורת העבודה המקורית - זו המתלה שעליו בעבר היינו תולים את הפעולה הבאה - וליצור ממנה ידית/פרומיס שתגיב בצורה נכונה להצלחה וכשלון של הפעולה, והיא תשתמש לנו כידית לתלות עליו עוד פעולות.
הבנאי של פרומיס מקבלת פונציה/פעולה שהיא מריצה, ובתוך הפונקציה כאשר קורה כשלון אתה אמור לאותת את זה על ידי קריאת reject ובמצב של הצלחה מאותתים את זה על ידי קריאה ל-resolve.

@dovid אמר בכתיבה נכונה של פונקציית executor עבור פרומיס:

זה מבלבל לי וזה גם סתם לדחוף לתוך פונקציית הפרומייז הקדמות בלתי נחוצות

זו בעיה אחת. ענין של טעם טוב.

להסביר יותר למוקשי הבנה כמוני או לאלה שהטעם הטוב מגיע להם אחרי התבוננות שכלית והוא עוד לא אינטואיטיבי אצלם:
הטעם הטוב של המתכנת אמור להורות לו שאם פונקצייה מסויימת אמורה לפעול פעולה א' - שלא לערב שם דברים אחרים. והרי פונקציית ההרצה של הפרומיס נועד לעשות רק דבר אחד: המרת פעולה callback-ית לפרומיס על ידי הרצת הפעולה ה-callback-ית ואיתות הצלחתה/כשלונה לידית/פרומיס. ואם כן אין להוסיף שם עוד דברים.
למה? כי כל מה שאתה מוסיף דברים שלא קשורים לענין - אתה בעצם מגרע. כי כבר לא ברור מה אתה רוצה לעשות.

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

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

הבעיה שעליו אני מדבר הוא כזה:
כאשר אתה מעביר פונקציה לבנאי של הפרומיס, הבנאי לוקחת את הפונקציה ומריצה אותה. אחרי ההרצה הבנאי מאזינה לאיתותים כדי לקבל מידע על הצלחתה/כשלונה של הפעולה. בעת קבלת איתות, הבנאי תגיב על ידי הפיכת מצבה של הפרומיס ל-fulfilled או rejected.
יש שתי צורות שהבנאי מקבלת איתותים אלו. א) על ידי פונקציות resolve\reject וב) על יד תפיסת חריגות. חריגה (exception) בקוד של הפעולה תגרום מיד ל-rejection של הפרומיס.

כדי שהקוד תצליח, אנחנו חייבים לקבל מצב שבו אין מוצא אחרת מהפונקציה - רק resolve או reject או exception.
אם משהו רביעי קורה, נישאר תקוע עם פרומיס "קפואה" שאף פעם לא נדע את מצבו, כי אין לנו דרך אחרת לאותת את מצב הפרומיס.

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

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

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

function getFooPromise () {
  return new Promise(async (resolve, reject) => {
    try {
      const bar = await getBar();
      bar.getFooCallbackStyle((error, data) => {
        if (error) return reject(error);
        resolve(data);
      });
    } catch (e) {
      reject(e);
    }
  });
}

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

(אם קראת עד לפה - שאפו...)