フロントエンド開発で学んだ命名の重要性

VIDEO BRAINのフロントエンドを担当している松井です。入社して早8ヶ月が経ち、毎月進化していくスピード感にようやく慣れつつあります。前回はVIDEO BRAINのフロントエンドの全体アーキテクチャについて書きましたが、今回はもっとコードレベルの事を書こうと思います。

f:id:open8tech:20200624200531p:plain

言わずと知れた名著リーダブルコード。

この本はコードは他の人が最短時間で理解できるように書くという考えの基に具体的なTipsを色々と紹介しています。

今回はこの考えの重要性をVIDEO BRAINの開発を通じて体験できたので、実例を交えながら書いていこうと思います。

コードは他の人が最短時間で理解できるように書く

よく言われている事ですが、開発時は書く時間より読む時間の方が長いです。

今回のテーマで海外記事も読みましたが、このフレーズを頻繁に目にしました。

Write readable code because it’s read more than it’s written.

複雑な処理などをしばらく経ってから読み返すと自分で書いたコードでも把握するのに時間がかかる事がよくあります。

プログラミングでありがちな数週間前の自分は他人現象です。

その原因の多くは関数が何をしてるのか？、変数に入ってる値は何なのか？を理解するのに時間がかかる事だと思います。だからこそ適切な名前をつけてラベリングしておけば、どこに何が入ってるのか、何をしてるのかが追いやすくなり、ラベルを見るだけで大体のイメージを掴む事ができます。

それでは開発中に体験した悪い例、いい例を紹介していこうと思います。

悪い例１：スコープの大きい値に抽象的な命名

子Componentに渡す様なスコープの大きい値にvalueという抽象的な名前で渡してしまっていた。

そのvalueの値が具体的に何なのか？は開発中は分かっていても、後から自分がそのコードを読んだ時に、「何を入れたっけ？」となり、わざわざ親Componentで確認しないとイメージがつかめなかった。書いた本人でもそうなのだから他のチームメンバーも同じ思いをしたはずです。

valueのような抽象的な命名はスコープの小さいものや、ローカル変数など必要な情報が近くにある時に留めるべきで、Propsの様な、ファイルをまたいで渡される値には必ず具体的な命名をすべきだと反省しました。

悪い例2：誤解を招いてしまう命名

isInvalid = true/false

上記の様なBooleanの変数を作って、isInvalidの場合にアラートを表示するという使い方をして問題なく動作していたのですが、チームメンバーから無効という名前なのに中身がtrueだと不自然で、誤解を生みやすいと指摘された事がありました。

isValid = true/false

にして !isValidの場合にアラートを出すという処理の方が自然言語的でいいという事でした。

変数名だけではなく、その中に入れる値、その使い方も考慮した上でコードを書かないといけないという学びになりました。

いい例1：UIがイメージできる命名

命名は変数や関数だけではありません。

VIDEO BRAINはstylingにstyled-componentsを導入していてコンポーネントに命名ができます。

以下のstyled-componentsを見ると、これがどんなUIなのかイメージがつくと思います。

<SeekBar>
     <SeekBarPlay>
          {isPlaying ? <PlayerPause /> : <PlayerPlay />}
     </SeekBarPlay>
     <SeekBarLine>
          <SeekBarLineProgress />
          <SeekBarLineCircle />
          <SeekBarLinePointingProgress />
     </SeekBarLine>
     <SeekBarLinePointingTime />
     <SeekBarSecond>
          <SeekBarSecondCurrent></SeekBarSecondCurrent>
          <SeekBarSecondDuration></SeekBarSecondDuration>
     </SeekBarSecond>
     <SeekBarSound>
          {isSoundOn ? <SoundOn /> : <SoundOff />}
     </SeekBarSound>
</SeekBar>

正解は動画のプレビュー画面の下にある再生ボタンやシークバーなどのUIです。

f:id:open8tech:20200624200600p:plain

構成もいいですが、命名が簡潔で一貫性があり、これを見ただけでどんなUIかイメージが湧きました。逆にこれをいい加減にしてしまうとどれが何のUIなのかいちいちビルドして確認しないといけなくなります。

こういう細かい気遣いが、他のチームメンバー、もしくは数週間後の自分が読むストレスを軽減するので、とても大事だと思いました。

いい例2：

durationToTimeFormat(duration)

この関数名を見た時に秒数を時計表示に変えていると一目で予想がつきました。 VIDEO BRAINのユーザーが作る動画の再生時間はバックエンド側では全て秒数で管理されていて、それを時計（00:00:00）表示に変換しているといういい命名だと思いました。

参考までに、durationToTimeFormatは実際こんな処理をしてます。

durationToTimeFormat = (time: number) => {
     const h = ('00' + parseInt((parseInt(time) % 86400) / 1440)).slice(-2);
     const m = ('00' + parseInt((parseInt(time) % 3600) / 60)).slice(-2);
     const s = ('00' + parseInt(parseInt(time) % 60)).slice(-2);
     return `${h}:${m}:${s}`;
};

個人的にこの関数名が、durationToTime()だったらそこまでピンとこなかったかもしれません。durationもTimeも同じ意味といえば同じなので、どんな変換をしたんだ？と思ってしまったかもしれません。

TimeFormatとした事で時刻フォーマットに変換したんだなというイメージが湧きました。

いい例3：

checkHasAudioTrack(video)

読んで字のごとく、動画に音があるのかどうかチェックする関数です。

単純にcheckAudioTrack()だったらAudioTrackの何をチェックしてるか分かりませんが、hasとつくとAudioTrackの有無をチェックしてると予想がつきます。

いい例4：

ページUIを改修した時の例です。もともと最大5ページを表示していたのですが、新たに最大９ページ表示に変更してほしいという指示でした。

表示は大きく分けて10ページ未満か以上かの2パターン

10ページ未満の場合は全ページを表示（最大9ページ） f:id:matsuihopen8:20200723120004p:plain

10ページ以上の場合、以下3種類の表示条件で分ける

・最初の5ページは9ページ固定で表示 f:id:matsuihopen8:20200723120021p:plain

・6ページ目以降は現在のページを中心に前後に4ページ、合計9ページを表示 f:id:matsuihopen8:20200723120032p:plain

・最後の5ページは9ページ固定で表示 f:id:matsuihopen8:20200723120041p:plain

その条件式を自分は最初にこのように書き、PRを出しました。 pageNum は合計のページ数 page は現在のページ pages という配列に表示するページの番号をPushするという処理です。

const pages = [];
if (pageNum < 10) {
  for (let i = 1; i <= pageNum; i++) {
    pages.push(i);
  }
} else {
  if (page < 6) {
    for (let i = 1; i <= Math.min(9, pageNum); i++) {
      pages.push(i);
    }
  } else if (pageNum - page < 5) {
    for (let i = pageNum - 8; i <= pageNum; i++) {
      pages.push(i);
    }
  } else {
    for (let i = Math.max(page - 4, 1); i <= Math.min(page + 4, pageNum); i++) {
      pages.push(i);
    }
  }
}

Reviewerから条件式内の数字に何の意味があるのか命名してくれないとわからない。配列にPushする処理は共通化した方がいいという指摘を受けて、以下のように修正しました。

const previous4Pages = page - 4;
const next4Pages = page + 4;
const last8Pages = pageNum - 8;
const showPageMax = 9;
const isFirst5Pages = page < 6;
const isLast5Pages = pageNum - page < 5;
let iterateStartNum = Math.max(previous4Pages, 1);
let iterateLimitNum = Math.min(next4Pages, pageNum);
if (isFirst5Pages || pageNum < 10) {
  iterateStartNum = 1;
  iterateLimitNum = Math.min(showPageMax, pageNum);
} else if (isLast5Pages) {
  iterateStartNum = last8Pages;
  iterateLimitNum = pageNum;
}
for (let i = iterateStartNum; i <= iterateLimitNum; i++) {
  pages.push(i);
}

条件式に使う各数字に命名して意味を持たせました。さらにループして配列へのPush処理を共通化させた事で可読性が増し、Reviewの負担が減ったと思いますし、自分自身も後から読んだ時に困る事がなくなりました。

いざ自分が変数、関数に命名するとなると簡単にできる時もあれば、なかなかいい言葉のチョイスが思いつかない時もあって大変ですが、これらのいい例を参考に、多少長くなっても読み手にとってストレスのない命名をする事が大事だと思いました。

コードの質の向上→生産性の向上→事業の成長

ソフトウェア開発において、一度作ったものをそのまま何ヶ月も使い続けることはありえない事で、ましてやVIDEO BRAINは毎月進化を遂げています。

「命名」は処理を書くロジカルな作業とは異なるクリエイティブな作業とも言えますが、コードの質を上げる為に疎かにできない作業ですし、この作業に少し余計に時間を使う価値はあると言われています。

コードの質が上がれば生産性が上がり、生産性が上がれば事業の成長スピードにも影響を与える。

競合サービスを出し抜いていくためには、開発時のこういう地道で小さな心使いの積み重ねが大きな差を生んでいくかもしれません。