דוגמת Refactoring ב React כדי לקבל Markup נקי יותר

28/07/2020

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

במילים אחרות אם כתבתי 2 בתיבת השעות אוטומטית יופיע 120 בתיבת הדקות ו 7200 בתיבת השניות. ואם כתבתי 60 בתיבת הדקות אוטומטית יופיע 1 בתיבת השעות ו 3600 בתיבת השניות. אני משנה טקסט בתיבה אחת והשתיים האחרות צריכות להתעדכן בהתאמה.

הרבה מאוד פעמים אנשים יפתרו את התרגיל עם קוד שנראה בערך כך:

function TimeConverter(props) {
    const [seconds, setSeconds] = useState(0);

    return (
        <div>
            <label>
                Seconds:
                <input
                    type="number"
                    value={seconds}
                    onChange={e => setSeconds(e.target.value)}
                />
            </label>
            <label>
                Minutes:
                <input
                    type="number"
                    value={seconds / 60}
                    onChange={e => setSeconds(e.target.value * 60)}
                />
            </label>
            <label>
                Hours:
                <input
                    type="number"
                    value={seconds / 3600}
                    onChange={e => setSeconds(e.target.value * 3600)}
                />
            </label>
        </div>
    );
}

וזה עובד אבל ה JSX הוא לא אידאלי. הבעיה הראשונה איתו היא הקוד הכפול (שלושת ה input-ים וה label נראים ממש דומה), והבעיה השניה ואולי יותר קשה היא הלוגיקה שמוחבאת בתוך ה JSX. כשמסתכלים על הקוד לא לגמרי ברור מה הקומפוננטה הזאת צריכה לעשות, ואנחנו צריכים לשבת ולפענח מה כל input עושה כדי שנוכל לבנות בראש תמונה מלאה.

אחרי ששמנו לב לשתי הבעיות אנחנו יכולים להתקדם לבצע Refactoring לקוד כדי לפתור אותן. אני מתחיל עם איך שהייתי מדמיין שהקומפוננטה צריכה להיות:

function TimeConverter(props) {
    const time = useTime(0);

    return (
        <div>
            <SecondsPanel time={...time} />
            <MinutesPanel time={...time} />
            <HoursPanel time={...time} />
        </div>
    );
}

אני לא יודע עדיין מה זה time ואין לי את שלושת הקומפוננטות אבל כבר אפשר לראות שהקוד השתפר: אין כפל קוד, ברור ששלושת הקומפוננטות מתיחסות לאותו "דבר" (שאני עדיין לא בטוח מהו), וברור שממיר היחידות הזה כולל שלושה פאנלים שנקראים "שניות", "דקות" ו-"שעות".

עכשיו אני יכול להמשיך לבנות את הקומפוננטות ואת time. נתחיל עם הפונקציה useTime: היא צריכה להחזיר משהו שאני יכול להעביר לקומפוננטות, משהו ששמור בסטייט של הקומפוננטה הראשית ומשהו שבאמצעותו הקומפוננטות יוכלו לעדכן את הקומפוננטה הראשית כשהערך משתנה. בקוד המקורי היה לי שם משתנה סטייט ופונקציה שמעדכנת אותו, ואני יכול להמשיך באותו מבנה:

function useTime(initialValue) {
    const [value, setter] = useState(initialValue);
    return {
        time: value,
        setTime: setter,
    };
}

בשלב ראשון אני יכול לכתוב את שלושת הקומפוננטות עם אותו קוד שהיה לי ב JSX הראשוני:

function Seconds(props) {
    const { time, setTime } = props;

    return (
                <label>
                    Seconds:
                    <input
                        type="number"
                        value={time}
                        onChange={e => setTime(e.target.value)}
                    />
                </label>
    );
}

function Minutes(props) {
    const { time, setTime } = props;
    return (
            <label>
                Minutes:
                <input
                    type="number"
                    value={time / 60}
                    onChange={e => setTime(e.target.value * 60)}
                />
            </label>
    );
}

function Hours(props) {
    const { time, setTime } = props;
    return (
            <label>
                Hours:
                <input
                    type="number"
                    value={time / 3600}
                    onChange={e => setTime(e.target.value * 3600)}
                />
            </label>
    );
}

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

function timeUnit(name, factor) {
  const TimeUnit = (props) => {
    const { time, setTime } = props;
    return (
      <label>
        {name}
        <input
          type="number"
          value={time / factor}
          onChange={e => setTime(e.target.value * factor)}
          />
      </label>
    )
  }
  TimeUnit.name = name;
  return TimeUnit;
}

ובאמצעותה ליצור את הקומפוננטות:

const Seconds = timeUnit('Seconds', 1);
const Minutes = timeUnit('Minutes', 60);
const Hours = timeUnit('Hours', 3600);

התוצאה היא קוד נקי בהרבה שגם אפשר בקלות להרחיב אותו לממירים מסוגים אחרים (למשל ממיר מטבעות):

function timeUnit(name, factor) {
  const TimeUnit = (props) => {
    const { time, setTime } = props;
    return (
      <label>
        {name}
        <input
          type="number"
          value={time / factor}
          onChange={e => setTime(e.target.value * factor)}
          />
      </label>
    )
  }
  TimeUnit.name = name;
  return TimeUnit;
}

function useTime(initialValue=0) {
    const [value, setter] = React.useState(0);
    return { time: value, setTime: setter };
}

const Seconds = timeUnit('Seconds', 1);
const Minutes = timeUnit('Minutes', 60);
const Hours = timeUnit('Hours', 3600);

function TimeConverter(props) {
  const time = useTime();

  return (
    <div>
      <Seconds {...time} />
      <Minutes {...time} />
      <Hours {...time} />
    </div>
  );
}

ReactDOM.render(<App />, document.querySelector('main'));

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