DRY/KISSで太りすぎた関数を救え！｜プリンシプルオブプログラミング実践編①

1. 次のコードは、「とりあえずなんでも詰め込んだユーティリティ関数」です。KISSの観点から、どのような改善方針がよいでしょうか？

function doMagic(obj, options) {
  // ログ出力
  console.log('start', obj, options);

  // バリデーション
  // ...

  // データ変換
  // ...

  // API呼び出し
  // ...

  // 画面更新
  // ...
}

A. さらに多くの処理を足して、アプリ全体をここから呼ぶ
B. doMagic の中身を、責務ごとに小さな関数に分割し、意味のある名前を付ける
C. すべての処理をコメントアウトしておく
D. 関数名だけ doEverything に変える

2. 次のコードでは、10という値が複数の場所で使われています。DRYの観点から、最も良い改善はどれでしょうか？

function isNewUser(user) {
  return user.daysSinceRegistered < 10;
}

function isRecentOrder(order) {
  return order.daysSincePlaced < 10;
}

A. 10 を 7 に変更する
B. 10 を定数として切り出し、意味のある名前を付けて共有する
C. "< 10" を "<= 10" にする
D. daysSinceRegistered と daysSincePlaced を削除する

3. 次のコードでは、バリデーションメッセージがコピペされています。DRYの観点から、どこを改善するとよいでしょうか？

function validateName(name) {
  if (!name) {
    return '名前は必須です';
  }
  if (name.length > 20) {
    return '名前は必須です';
  }
  return '';
}

A. 20 の制限をなくす
B. エラーメッセージをそれぞれ別の文言にする
C. 同じメッセージを返している部分をまとめ、条件だけ分岐させる
D. validateName 関数を削除する

4. 次の関数は、画面ごとにほぼ同じローディング表示ロジックをコピペしています。DRYの観点から、最も適切な改善はどれでしょうか？

function showUserLoading() {
  const el = document.getElementById('user-section');
  el.innerHTML = '読み込み中...';
}

function showOrderLoading() {
  const el = document.getElementById('order-section');
  el.innerHTML = '読み込み中...';
}

A. innerHTML を textContent に変える
B. 'loading' クラス名を変える
C. ローディング表示用の共通関数を定義し、IDだけ引数で渡す
D. ローディング表示をやめる

5. 次の関数は、かなり多くの責務を抱えて太りすぎています。DRY/KISS の観点から、最も問題がある点はどれでしょうか？

function handleCheckout(cart, user) {
  // 合計計算
  // クーポン適用
  // 在庫チェック
  // 配送先住所チェック
  // 支払情報チェック
  // 決済API呼び出し
  // 注文レコード作成
  // メール送信
  // 画面遷移
}

A. コメントが多すぎること
B. 引数が2つあること
C. チェックアウトに関するすべての処理を1つの巨大な関数に詰め込んでいること
D. 関数名に handle が含まれていること

6. 次の関数は、バリデーション・保存・画面遷移までを1つに詰め込んだ例です。KISSの観点から、最も優先して行うべき改善はどれでしょうか？

function submitForm(data) {
  // 入力チェック
  if (!data.name) {
    alert('名前は必須です');
    return;
  }
  if (!data.email) {
    alert('メールアドレスは必須です');
    return;
  }

  // 保存用データ整形
  const payload = {
    name: data.name,
    email: data.email,
    createdAt: new Date().toISOString()
  };

  // 保存API
  fetch('/api/contacts', {
    method: 'POST',
    body: JSON.stringify(payload)
  }).then(() => {
    alert('送信しました');
    location.href = '/thanks';
  }).catch(() => {
    alert('送信に失敗しました');
  });
}

A. createdAt を削除する
B. fetch を同期処理にする
C. バリデーション・保存ロジック・画面遷移を役割ごとに分割し、小さな関数にする
D. location.href の代わりに console.log を使う

7. 次の2つの関数は、ほとんど同じログ出力を行っています。DRYとKISSの観点から、より良い設計はどれでしょうか？

function logUserLogin(user) {
  console.log('[LOGIN]', new Date().toISOString(), user.id, user.name);
}

function logUserLogout(user) {
  console.log('[LOGOUT]', new Date().toISOString(), user.id, user.name);
}

A. ログ出力をすべて削除する
B. 日付の出力を削除する
C. 共通の logUser(eventType, user) 関数を作り、[LOGIN]/[LOGOUT] だけを引数で変える
D. console.log の代わりに alert を使う

8. 次の if 文は、KISSの観点から少し読みにくい書き方になっています。最も改善したいポイントはどれでしょうか？

function canShip(order) {
  if (order && order.paid && !order.canceled && !order.refunded && order.items && order.items.length > 0) {
    return true;
  }
  return false;
}

A. order を引数に渡していること
B. 1つの if 条件に多くのチェックを詰め込みすぎていること
C. !order.canceled と !order.refunded を使っていること
D. return true / false を使っていること

9. 次のコードは、API呼び出しのオプションを毎回ベタ書きしています。DRYの観点から、最も効果的な改善はどれでしょうか？

function fetchUsers() {
  return fetch('/api/users', {
    method: 'GET',
    headers: { 'Content-Type': 'application/json' },
    credentials: 'include'
  });
}

function fetchOrders() {
  return fetch('/api/orders', {
    method: 'GET',
    headers: { 'Content-Type': 'application/json' },
    credentials: 'include'
  });
}

A. method を 'POST' に変更する
B. 'Content-Type' をヘッダーから削除する
C. 共通の fetch オプションを変数や関数にまとめ、URLだけ変えるようにする
D. fetch を axios に置き換える

10. 次の関数は、複数のモードに応じて処理を切り替えています。KISSの観点から、最も問題になりやすい点はどれでしょうか？

function handleAction(mode, data) {
  if (mode === 'create') {
    // 作成処理
    // ...
  } else if (mode === 'update') {
    // 更新処理
    // ...
  } else if (mode === 'delete') {
    // 削除処理
    // ...
  } else if (mode === 'copy') {
    // コピー処理
    // ...
  } else if (mode === 'archive') {
    // アーカイブ処理
    // ...
  } else {
    throw new Error('Unknown mode');
  }
}

A. mode を文字列で管理していること
B. 例外を投げていること
C. 1つの関数で多くのモードを扱い、if-else が増え続ける設計になっていること
D. data を引数に取っていること

コードレビュー道場 #1
DRY/KISSで太りすぎた関数を救え！

目次

コードレビュー道場 #1～DRY/KISSで太りすぎた関数を救え！～

2. 次のコードでは、10という値が複数の場所で使われています。DRYの観点から、最も良い改善はどれでしょうか？

`function isNewUser(user) { return user.daysSinceRegistered < 10; } function isRecentOrder(order) { return order.daysSincePlaced < 10; }`

DRY/KISSで太りすぎた関数を救え！
採点結果

参考文献・出典

このページの著者

もちもちみかん.comとは

コードレビュー道場 #1DRY/KISSで太りすぎた関数を救え！

目次

コードレビュー道場 #1～DRY/KISSで太りすぎた関数を救え！～

2. 次のコードでは、10という値が複数の場所で使われています。DRYの観点から、最も良い改善はどれでしょうか？ function isNewUser(user) { return user.daysSinceRegistered < 10; } function isRecentOrder(order) { return order.daysSincePlaced < 10; }

参考文献・出典

このページの著者

シェアしてね

コードレビュー道場 #1
DRY/KISSで太りすぎた関数を救え！

2. 次のコードでは、10という値が複数の場所で使われています。DRYの観点から、最も良い改善はどれでしょうか？

`function isNewUser(user) { return user.daysSinceRegistered < 10; } function isRecentOrder(order) { return order.daysSincePlaced < 10; }`