【コードレビュー】 一見同じ値に見える引数の定義

業務上のコードレビューで気になったものの中から「一見同じ値に見える引数の定義」（ニホンゴムズカシ〜）についてまとめてみようと思います。

前提

指摘があったケースは関数内で次のような項目の登録処理をしていました。

• 書類に記載する日付
• 書類の送信日時
• 書類の登録日時

そして書類データはこの関数を呼出した時点で作成されます。

レビューの最初の時点で関数内では呼び出された日時をこれら3つの項目に登録するようにしていました。

export const create = async (key: DataKey) => {
  const documentData = {
    key,
    date: new Date(),
    submittedAt: new Date(),
    registeredAt: new Date(),
  };
  return await save(documentData);
};

ですが、テストケースなどを考えるとこれらの項目はオプション引数として指定できる方がいいという指摘がありました。

NG

実際の動きとしてはどれも現在日時を使っていたので、オプション引数としてデフォルトで現在日時を取るような引数を定義してしまいました。

export const create = async (key: DataKey, options?: { documentDate?: Date }) => {
  const documentDate = options?.documentDate ?? new Date(); // 何も指定しない場合は現在日時
  const document = {
    key,
    date: documentDate,
    submittedAt: documentDate,
    registeredAt: documentDate,
  };
  return await save(document);
};

レビュイーは3つの項目が現在日時を登録するものだという考えで引数名をdocumentDateにしました。
ただ、それぞれ別の値を指定するようにこの関数を呼び出したい場合この定義だと対応できません。

OK

3つの項目は目的が違うので、それぞれ指定できるようにしました。
指定されていない場合は現在日時をデフォルト値として使うようにしています。
関数を呼び出す側の仕様変更にも対応できるし、テストケースで色々な値を試したい場合も対応できます。

export const create = async (
  key: DataKey,
  options?: {
    documentDate?: {
      date?: Date;
      submittedAt?: Date;
      registeredAt?: Date;
    };
  },
}) => {
  const defaultDocumentDate = new Date(); // 何も指定しない場合は現在日時をデフォルト値として使う
  const documentDate = options?.documentDate;
  const document = {
    key,
    date: documentDate?.date ?? defaultDocumentDate,
    submittedAt: documentDate?.submittedAt ?? defaultDocumentDate,
    registeredAt: documentDate?.registeredAt ?? defaultDocumentDate,
  };
  return await save(document);
};

まとめ

今回は同一の書類データに登録するということで、分ける意味ある？となるかもしれませんが、別の種類のデータのための値の場合は明確に分ける必要があると思います。
1つの値を変更したら芋づる式にあちこちの値に影響するようなコードはデグレの可能性が高くなるので避けるようにしたいですね。