Lycolog

今までLLMを使う場合、文書校正や整理、ERP辺りが多かったが、そろそろコード作成にも必要だなと感じたので取り組んでみた結果の初回の雑感。

Claude Opus 4との直接対話

LLMエージェントを使わない、チャットインターフェースでの直接対話で行ってみたこと。これはClaude Opus 4で行っている。

簡単なボイラープレートやプログラムが関の山

正直、LLMとの単純な対話で作れるのは3カラムのハンバーガーメニュー付きのような画面のボイラープレートや、WebでJSを使った画像判定スクリプトあたりが関の山だと感じている。

それ以上のものも作れる可能性はあるが、要件定義とコードレビューが大変なので厳しい気がしている。

プログラムの変換は苦手

まず私はTampermonkeyで5分ごとにAPIをポーリングし、結果をパースして条件に応じてOSに通知トーストを出す、400行ほどのスクリプトを作っている。

そこで、このソースコードを丸っと渡して、C#.NETに変換してほしいと頼んでみたが、これは失敗した。根本的にビルドが通らないコードが出てきて多少の修正でどうにかなるレベルでもなく、全くダメだった。

ファイル構成もよくなく、ModelやControllerレベルではファイル分割されているものの、1ファイルの中に複数クラスが納められていたり、何ともな結果だった。

TSDocを書いているため、上手く推論できればInterfaceやClassも作れると思ったが、これは難しいようだった。

特定の設定方法を書くのは得意

OpenWrtの特定の設定を書かせることは得意だった。これはそのまま適用できた。やはりスコープが限定されているのが得意だと感じた。

Claude Codeを少しつついてみた感想

ファイル保存などの手間がいらなくなる

当たり前だがローカルマシン上に結果を出力するため、チャットインターフェースのように頑張ってファイルを保存したり、ディレクトリを切る必要は全くなくなる。

ボイラープレートの作成は得意

PHPを利用したMVC構成で簡単なブログをフルスクラッチで作ってほしいといえば、それらしい形のものは出してくれた。

動くかどうかは全く試していないが、大まかなスケルトンを作って貰って、そっからいじっていくベースとしては使えるような気がした。

やっぱりプログラムの変換は苦手

adiaryのテンプレートエンジン部分をPHPに書き換えてほしいと依頼してみたが、やはり動かないものが出てきた。adiaryの設計が極めて複雑でコンテキストが読み取りづらいのはあると思うが、やはりこの手の作業は苦手なようだ。

現状で見えてきたこと

そこまで大して使ったわけではないが、とりあえず所感として。

恐らく小規模でコンテキストの薄いコードを書かせるのが筋がよさそう。これは複雑な要件をLLMに伝えるのは難しいし、考えるのも大変なのと、コード変換も400行レベルでも厳しいと感じたからだ。

つまり、既存システムの移行は苦手なのではないかと思っている。なのでWordPressをGoで作り直すみたいなことは相当難しいと思う。逆にSOLID原則やClean Architectureのような、スコープが狭く責務が明確なものは作りやすいのではないかと感じた。

また仮にLLMが全て書いてくれるとしても、人がレビューしないとバグがあった時に当たりをつけるのが大変とか、知らない仕様が紛れ込んだりとかもあるため、LLMに書かせすぎるべきではなく、あくまで補助ツール程度に留めておくのが良いと考えている。

LLMの制約を味方にする開発術という記事を見た感じ、複雑なタスクを段階的に分解し、LLMの処理可能な単位に分解することが重要だと感じている。つまりこれは疎結合のほうが向いているということだ。また標準化されていて、属人性がないコードのほうが制約が少なくなるので、LLMもやりやすくなるだろう。これは標準化されておらず、属人性が高いコードは往々にしてカオスで、判断軸がなく、LLMの思考がぶれるからだと思われる。

結局どうしていくか

正直まだどう実用化していくかの展望は見えていない。

何はともあれ使い続けていくことが大切な気はしているので、個人的にはClaude Codeを使い続けていきたい。少なくとも面倒なボイラープレートを書く部分については非常に優秀なので、大まかに作らせて微調整するみたいな用途では間違いなく活路がある。こういうのは引き出しが多ければ多いほど活用できるだろうから、基礎を忘れないように自学していくことも引き続き重要で、LLMに教えてもらうのもいいだろう。適切に使えばLLMからは多くの学びを得られる。

自動生成されたHTMLから社員情報を引っこ抜くスクリプトを作った時に考えたことを書き出してみる。ブラウザのDevToolsを使った簡易ツールを書いていて、頭に浮かんだことだ。データやコードは架空の内容なので実在しない。

• ベースのHTML
• まずは関数ベースで考える
• クラスにしてみる
• 追加仕様に合わせ関数を新設し、クラスを改良する
• 更により良くするには？
• クラスにするか、関数にするか

ベースのHTML

大まかに最初に目に入ってきたのはこんな内容だった。機械生成されているせいかフォーマットがガタガタ気味だ。

<span class="Content">
    <span id="undefined">SHAIN.XLS page1</span>
</span>
<span class="Content">
    <br>
    <span id="undefined">社員リスト</span>
    <span id="undefined"></span>
</span>
<span class="Content">
    <br>
    <span id="hoge1">00001</span>
    <span id="fuga1">：山菱喜一</span>
</span>
<span class="Content">
    <br>
    <span id="hoge2">00002</span>
    <span id="fuga2">：鈴木与一</span>
</span>
<span class="Content">
    <br>
    <span id="hoge3">00011</span>
    <span id="fuga3">：茨城妙子</span>
</span>
<span class="Content">
    <span id="undefined">SHAIN.XLS</span>
    <span id="undefined"> page2</span>
</span>
<span class="Content">
    <br>
    <span id="hoge4">00012</span>
    <span id="fuga4">：奈良楓</span>
</span>
...

まずは関数ベースで考える

ひとまず普段は関数ベースで実装しているので関数ベースで実装する。書いていてgetEmployee(name, list)の仮引数にあるlistは冗長ではないかということに気づく。

const emp_list = document.getElementsByClassName('Content');
[...emp_list].reduce((acc, cur) => {
  if (cur.id === 'undefined' || cur.children.length !== 3) {
    return acc;
  } else {
    const id = cur.children[1].innerText;
    const name = cur.children[2].innerText.replace(/^：/, '');
    if (/^\d+$/.test(id)) {
      acc.push({ id, name });
    }

    return acc;
  }
}, []);

const getEmployee = (name, list) => {
  const re = new RegExp(`.*${name}.*`);
  const result = list.filter((emp) => re.test(emp));
  console.log(result);
};

クラスにしてみる

そしてクラスにしたほうが素直にならないかと考え、クラスにしてみた。getEmployee(name, list)はsearch(name)となり、冗長性が減ったほか、抽象的な表現で分かりやすくなった気がする。

class EmployeeSearch {
  constructor() {
    const emp_list = document.getElementsByClassName('Content');
    this.list = [...emp_list].reduce((acc, cur) => {
      if (cur.id === 'undefined' || cur.children.length !== 3) {
        return acc;
      } else {
        const id = cur.children[1].innerText;
        const name = cur.children[2].innerText.replace(/^：/, '');
        if (/^\d+$/.test(id)) {
          acc.push({ id, name });
        }

        return acc;
      }
    }, []);
  }

  search(name) {
    const re = new RegExp(`.*${name}.*`);
    const result = this.list.filter((emp) => re.test(emp.name));
    console.log(result);
  }
}

追加仕様に合わせ関数を新設し、クラスを改良する

動作確認をしていく中で表示と名前が分かれたパターンを発見したので、これに合わせて直してゆく。

<span class="Content">
    <br>
    <span id="hoge2">00002</span>
    <span id="fuga2">：飯田</span>
    <span id="piyo2">太郎</span>
</span>

この対応をしようとすると分岐が必要だと考えたが、愚直にやってしまうとコンストラクタの中がごちゃつく上、テストもしづらいと考え、DOMをパースする処理を外部関数に切り出し、それを呼ぶことにした。

const getEmployeeFromList = (domList) => {
  const id = domList[1].innerText;
  const name1 = domList[2].innerText.replace(/^：/, '');
  if (domList.length === 3) {
    return {
      id,
      name: name1
    }
  } else {
    const name2 = domList[3].innerText;
    return {
      id,
      name: `${name1}${name2}`
    }
  }
};

class EmployeeSearch {
  constructor() {
    const emp_list = document.getElementsByClassName('Content');
    this.list = [...emp_list].reduce((acc, cur) => {
      if (cur.id === 'undefined' || cur.children.length < 3) {
        return acc;
      } else {
        const { id, name } = getEmployeeFromList(cur.children);
        if (/^\d+$/.test(id)) {
          acc.push({ id, name });
        }

        return acc;
      }
    }, []);
  }


  search(name) {
    const re = new RegExp(`.*${name}.*`);
    const result = this.list.filter((emp) => re.test(emp.name));
    console.log(result);
  }
}

更により良くするには？

ブラウザのDevToolsにあるConsoleで書いたコードなので、現実問題テストコードも書かないし、そこまで凝る必要もないのが、よりよい設計にするにはコンストラクタでパースせず、事前にパース処理した結果をコンストラクタを通じて注入して処理するようにするとよいだろう。そうすると責務が分離され、テストがしやすく、コードもきれいになると考える。

クラスにするか、関数にするか

これはまだ正直答えが見えていないが、インスタンスを引き回す場合はクラスのほうが都合がいい可能性がある気がしてきている。

関数でもできるにはできるが、毎回引数にインスタンス相当のものを渡すのは冗長である可能性がある。疎結合という文脈では関数のほうがより良い可能性もあるが、人間の認知負荷や、保守性との兼ね合いだと思う。

明確な損益分岐点はまだ見えていないし、今回のケースだと割とどっちでもいい部分もあるとは思う。

例外設計についての個人的に思っていることを書き出してみる。整理できていないがいったん現状。TypeScriptのケースを意識して考えているが、根幹は例外スローのある言語ではどれも共通だと考えている。

• 例外とは何か？
• 例外を多用しない
• 例外は構造化されたプログラムを破壊する
• 例外は処理コストが重い
• いつ例外を使うか？
• フレームワークやライブラリの例外をどう扱うか？
• カスタム例外を使う
• 例外を握りつぶさない
• スローする場合は、例外クラスを用いる

例外とは何か？

本記事で扱う例外とはthrow new Error("ほげほげ");のように、いわゆるスローされ、try-catchされるものを指して扱う。

例外を多用しない

例外は多用しないことが望ましく、原則として処理が続行不能になる場合を除き、使わないほうが良いと考えている。これはスローされた例外は可視化されづらく、適切にハンドリングされないケースがよくあるからだ。

例外は構造化されたプログラムを破壊する

構造されたプログラムとは順次・反復・分岐の基本構造を階層的に組み合わせたものであり、いわゆる上から下に読めばわかるもの、構造化プログラミングによって作られるものだ。上から下に流れるため非常にわかりやすい。

これに対して存在するのがgotoを用いたプログラムだ。gotoを使うとコードの色んな場所に前後しながらジャンプすることが可能で、可読性を損ねてしまう。

そして例外は基本的にgotoのような存在である。例外が起きると例外オブジェクトが投げられ、これはどこに行きつくか予想することが難しい。行きつく先がなければ最悪プログラムがクラッシュする恐れすらある。

例外は処理コストが重い

恐らく大抵の言語において例外をcatchする行為はコストが重い。これはJavaでは特に有名な話だと思うが、Java以外でもそうだと思う。例えばNode.jsでtry-catchで例外を処理するプログラムと、if-elseで処理するプログラムを書き、その実行速度を比較するとif-elseの方が早い。

以下はエラーハンドリングをifで行うプログラムと、try-catchで行うプログラムを作り、100万回走行させたときの処理時間だ。catchに入るケースでは処理速度が低下することが分かる。例外が投げられず、tryの中に納まる限りは低下しない。

これは例外が投げられる場合、最寄りのcatchポイントを探索するのに時間がかかるからではないかと考えている。

Node.js向け検証コード

前述の処理時間を出すのに使ったプログラム

// ==== 実行用共通部品
const execIf = (input) => {
  if (input === 1) {
    return true;
  } else {
    return false;
  }
};

const execThrow = (input) => {
  if (input === 1) {
    return true;
  } else {
    throw new Error('ERR');
  }
};

// ==== コールバック処理計測用関数
const getExecFuncElapsed = (cb) => {
  const startAt = +new Date();
  for (let i = 0; i < 1_000_000; i++) {
    cb();
  }
  return +new Date() - startAt;
};

// ==== If/try-catch検証用、コールバック処理群
const execIfOk = () => {
  const ret = execIf(1);
  if (ret) {
    console.log('OK=');
  }
};

const execIfNg = () => {
  const ret = execIf(0);
  if (ret === false) {
    console.log('ERR');
  }
};

const execThrowOk = () => {
  try {
    execThrow(1);
    console.log('OK=');
  } catch (e) {
    console.log('ERR');
  }
};
const execThrowNg = () => {
  try {
    execThrow(0);
    console.log('OK=');
  } catch (e) {
    console.log('ERR');
  }
};

// ==== 計測処理
const elapsedIfOk = getExecFuncElapsed(execIfOk);
const elapsedIfNg = getExecFuncElapsed(execIfNg);
const elapsedThrowOk = getExecFuncElapsed(execThrowOk);
const elapsedThrowNg = getExecFuncElapsed(execThrowNg);

// ==== 結果出力
console.table({ elapsedIfOk, elapsedIfNg, elapsedThrowOk, elapsedThrowNg });

いつ例外を使うか？

基本的には大域脱出がしたいケースのみで使うべきだろう。最悪ハンドリングされずとも、基底階層でcatchされれば、それでよいケースで使うのが最も無難だと考える。

大域脱出とは要するにgotoだ。多重ネストや深い高階関数から浅い階層に一気にすり抜けるときに有効だろう。逆に一階層とか、抜ける階層が浅いレベルでは使わないほうが良い。

例えば処理が続行不能になったケースでは例外をスローし、基底階層でcatchし、エラーログを吐く、クライアントに対してエラーメッセージを返すなどの処理があればよいと考える。

但し例外を使うときは極力カスタム例外を使ったほうがよいと考える。

フレームワークやライブラリの例外をどう扱うか？

例えばバリデーションエラーやHttpClient系の4xx, 5xx系の例外スローはラッパーを作り、例外をエラーオブジェクトに変換するのも一つだと考える。

HTTP GETを行うクライアント関数であれば以下のようなラッパーを作り、正常時は正常レスポンスを返し、異常時でかつ想定内であればエラーオブジェクトを返す、そして想定外の例外であればリスローする、といった処理をすることができる。

const httpGet = (url) => {
  try {
    return fetch(url);
  } catch (e) {
    if (e instanceof AbortError) {
      return createHttpErrorObj(e);
    } else {
      throw e;
    }
  }
}

こうすることで呼び出し元は正常時であればHTTPリクエストをそのままステータスコードに応じて処理し、Abortされた場合はリトライ、完全に予期せぬ内容であれば例外を基底階層まで飛ばして処理を中断するといったこともできる。

リトライに規定回数失敗した場合は、この関数の呼び元でnew OutOfTimesHttpRetryError()みたいな例外をスローするとよいだろう。

カスタム例外を使う

カスタム例外とは例外クラスを継承した例外クラスだ。

例えばC#ではExceptionのほかに、それを継承したSystemExceptionや、IndexOutOfRangeExceptionなど、様々なカスタム例外が存在する。

LaravelにもAuthorizationExceptionをはじめとし、多様なExceptionが存在する。

これらに限らず、大抵の言語やフレームワークには相応のカスタム例外が用意されているのが一般的で、業務システムやC2のプロダクトでも、例外をスローするケースではカスタム例外を作ることが望ましいと考える。

カスタム例外があると例外種別ごとにフィルタリングすることが可能になり、柔軟にハンドリングしやすく、ロギングの際にも例外種別を記録することで、後々の障害調査でも便利になるため有用だ。

例外を握りつぶさない

例外は時として握りつぶされることがある。そういう必要があるケースも少なからずあるだろう。しかし基本的に例外は握りつぶさないほうがよい。

例外は望ましくない現象が起きているはずで、本質的にハンドリングする必要がある。単に握りつぶしているだけでコメントやテストも書かれていなければ、もし例外が起きたとき、処理が正常に進むのかどうかコードから読み取ることが困難だ。

仮にテストがあったとしてもコードを読むときのコストが増えるので、基本的に握りつぶさないほうがよいだろう。

スローする場合は、例外クラスを用いる

JavaScript系では以下のようなエラーオブジェクトを作るケースもあると思うが、スローする場合は使わないほうが良い。

{
  __type: 'HogeError',
  message: 'fufagfuga',
  payload: someObject
}

スローする場合はErrorクラスを継承したカスタムエラーをスローすべきだ。これはカスタムエラーにはスタックトレースなど、障害調査をするときなどに例外として標準的な情報が格納されているほか、instanceofでフィルタしやすい、クラスは型情報を持つのでリファクタが容易、error.__type === 'HogeError'は言語機能の再実装に近く標準的ではないからだ。テストフレームワークでも例外をキャッチするためのアサーションではErrorクラスの継承が必須となっているケースがあるため、TypeScriptでは型検査が上手く通らないケースがある。

リーダーをするときに意識してることを簡単に書いてゆく。大まかには開発リーダー（チームリーダーレベル）の話。

先を見て動く

先を見て詰まりそうな要素があれば、関係各所と調整するなどで予め詰まらないように動いておくことで、メンバーの手を止めないように立ち回ることができる。

もし詰まることがあればメンバーを遊ばせることになるし、メンバーも好調にやっていたものがいきなり詰まれば不満や不信感を覚えるので、モチベーション低下につながり、これはプロジェクト運営上あまりよくない。

他にもメンバーの懸念を定期的に聞き取り、つぶしておくのも有用だと思っている。

メンバーの心理的安全性を保つ

メンバーの言うことは基本的に信頼し、否定しない。もし改善できるようであれば肯定したうえで、よりよい手法を示す。相手が腹落ちするまで話すなどで信頼を得る。

信頼が得られている場合、メンバーは言いたいことを言えずに抱え込んだり、裏で愚痴を言ったりといった行動が減り、言ってくれるようになるためそういった意見を受け止めて、プロジェクトを改善することができる。

特に相手の気を削ぐ、言葉を言わないように気を付けている。例えば「このプロジェクトは特殊だから」「そういう決まりだから」「どうしようもないことなんだ」はその一例だ。

暇にしておく

リーダーがパンクしているとメンバーが声を掛けづらかったり、声を掛けられても十分に対応できなくなることがある。結果としてメンバーの信頼を失うと、心理的安全の喪失やメンバーのモチベーション低下が起きるため、極力暇にしておく。全く何もしないというわけではなく落ちてるボールを拾ったりレビューに参加したり、今後のことを考えたり、締め切りのないタスクをやっていく感じだ。

具体的にどう暇にするかというとサブリーダーを立て権限を委任する、メンバーにも可能な範囲で権限を委任しておくなどで、自分で極力タスクを持たないようにする。

勿論いきなり委任してもメンバーはこなせないことがあるため、こなせるようにあらかじめ必要な知識をレクチャーしておくのがいい。例えば適切な設計や実装方法を何故それをするのかという意図から話しておくことで、設計作業を委任できる。

サブリーダーが必要な場合ではMTGに同席してもらい、動きを見てもらう、次に徐々に自分の代わりに意見してもらったり、タスク振りをしてもらうなどで慣れていってもらうことで、自分の役割を委任することができる。

間接的に評価する

メンバーのモチベーションが高いほど作業がスムーズに進むため、普段からモチベーションを保ってもらうために間接的な評価を行う。

例えば誰か別の人と話すときに「Aさんはとても作業品質が良い」などと評価を伝えておくと本人が何かの切っ掛けで小耳にはさんだ時にモチベーションが上がるきっかけになるため、誰かを評価できるシーンがあれば積極的にしておく。

こうしておくことで回りからも、その人物の評価が上がり、本人が小耳にはさむことがあれば直接評価よりも自信が持てると考えている。

今回は実際の開発現場で遭遇しそうなコードパターンを2つ挙げ、それらを主に可読性向上と保守コスト削減の視点から、どのように改善できるかを具体的に書いてゆく。

サンプルコードはTypeScript + GraphQLを使ったAPサーバーを想定して書いている。

• 命名とデータ構造の最適化
  • 元コード
  • 改善後コード
  • まとめ
• 型安全で見通しの良いエラーハンドリング
  • 元コード
  • 改善後コード

命名とデータ構造の最適化

この項目ではエラーメッセージを返すサーバー側の処理について記述する。

元コード

resolvers/customer-info/helpers/invalid-reason.ts

type ReferencePageLinkMapKey = 'userFormIssue' | 'duplicateUserName' | 'someErrorMessage';

// exportされているがgetInvalidInfo(key, boolean)でreferencePageLinkMap.get(key)が返ってくることを確認するテストに使われているだけ
export const referencePageLinkMap: Map<ReferencePageLinkMapKey, string> = new Map([
  ['userFormIssue', 'https://help.example.com/23843ddvwa'],
  ['duplicateUserName', 'https://help.example.com/xdxdk2t5'],
  ['someErrorMessage', 'https://help.example.com/pp33s'],
  ...
]);

const invalidReason = {
  inputIssue: {
    message: '入力項目にエラーがあります。...',
    links: [
      {
        label: '【登録時のヘルプページをご確認ください】',
        url: referencePageLinkMap.get('userFormIssue') ?? '',
      },
    ],
  },
  ...
};

export const getInvalidInfo = ({
  errorReasonType,
  isModeA,
}: {
  errorReasonType: ErrorReasonType;
  isModeA: boolean;
}): Omit<SomeFormInvalidInfo, 'invalidReason'> => {
  switch (errorReasonType) {
    case 'INVALID_INPUT':
    case 'SOME_INPUT_ERROR':
      return invalidReason.inputIssue;
...
   };
}

このコードの問題点

• 定義する必要性が希薄な型情報があり、改修コストが高い
• 変数や関数名、キー名などから相関する機能の関連性を読み取りづらい
• 定義されたスコープを超えて利用されているもので命名から機能の読み取りが難しいものがある
• リテラルがべた書きされており将来のリファクタリングに支障がある
• 不必要にMapが使用されており、実装が冗長になっている
• 静的な値の取得にNull合体演算子が使用されており、潜在不具合となりやすい
• 型情報がべた書きされすぎており、見通しが悪い
• 複数のフラグ値の正規化を利用側で行っており、複雑になっている

改善後コード

リソースファイルと実装が一緒くたであると読みづらいためファイルを分割する。パスも書いているがパスについては深く考慮できていない。

resources/messages/CustomerError.ts

export const ERROR_MESSAGES = {
  formValidationError: {
    message: '入力項目にエラーがあります。...',
    helpLinks: [
      {
        label: '【登録時のヘルプページをご確認ください】',
        url: 'https://help.example.com/23843ddvwa',
      }
    ],
  },
  ...
} as const;

まずtype ReferencePageLinkMapKeyは無駄に型の管理コストが増えるだけなので削除する。意味がないとは言わないが、型の追加削除によって発生するデグレードは普通は型検査でわかるためなくてよいだろう。

次にreferencePageLinkMapでは意味がわからないのでERROR_MESSAGESにリネームする。中身のキーもinputIssueでは分かりづらいのでformValidationErrorとし、フォームのバリデーションエラーであることが分かるようにする。linksについても何のリンクなのかわからないためヘルプのリンクであることを明示できるようにリネームする。

helpLinks.urlについては、共通のURLを参照する概念は普通ないか、あったとしても別に切り出して管理するのはコストなので直に書いてよいと考える。元のコードでもテストコード以外には使われていない。テストコードには実値を書かないと回帰テストとして機能しないため、この方式のほうがより良いと考える。

例えば以下のようなテストコードは回帰テストの観点では意味が薄い。

it('INVALID_INPUTの時にurlが正しい結果になること', () => {
  const invalidInfo = getInvalidInfo('INVALID_INPUT', false);

  expect(invalidInfo.links[0].url).toBe(invalidReason.get('inputIssue'));
});

これは以下のように書くことでページのURLが変更されたときにテストが失敗するため、より価値のあるものになる。但しこれはexpect(invalidInfo).toStrictEquals({ ... })の形で一括判定したほうが、テストコードの可読性の観点からより良いだろう。

it('INVALID_INPUTの時にurlが正しい結果になること', () => {
  const invalidInfo = getInvalidInfo('INVALID_INPUT', false);

  expect(invalidInfo.links[0].url).toBe('https://help.example.com/23843ddvwa');
});

ERROR_MESSAGESをリソース変数としてみなせば、これはべた書きというより定数定義とみなせるため、基本的には問題ないと考える。勿論これは同じURLを持つものが大量にあるなど、ケースによっては考慮の余地はあるだろう。

またMapをやめ、敢えて直値を書くことで万一存在しなかった場合に、元にあった以下の空文字が返ってきて機能しなくなる問題も解決している。そもそも通る余地がないロジックなので存在しなくてよい。

url: referencePageLinkMap.get('userFormIssue') ?? ''

またERROR_MESSAGESにas constを付与することで、意図しない破壊が起きる可能性を減らすことができる。

handlers/error/GetCustomerInfoError.ts

export const getCustomerInfoError = (errorType: CustomerErrorType) => {
  switch (errorType) {
    case CustomerErrorType.INVALID_INPUT:
    case CustomerErrorType.SOME_INPUT_ERROR:
      return ERROR_MESSAGES.formValidationError;
...
   };
}

まずgetInvalidInfo()をgetCustomerInfoError()にリネームすることで意味を分かりやすくしている。次に複雑な型情報を除去してシンプル（= (errorType: CustomerErrorType) =>）にする。errorTypeはerrorReasonTypeとisModeAを合成した結果を渡すことを想定している。これによって、この関数内の複雑性を減らすことができる。またCustomerErrorTypeはENUMなので、case 'CODE1':のようにリテラルべた書きを回避できるうえ、仕様変更などで値が変わった時にも対応がしやすくなる。

また戻り値の型も削除する。これは静的解析により自明であるほか、Omit<SomeFormInvalidInfo, 'invalidReason'>とあるが、実際に返しているのはinvalidReasonであり、関連性がないためだ（おそらく、偶々よく似た型を転用しているのだろう）。またこの事例では更にinvalidReasonという変数名があり、Omitの内容と視覚的に競合し、コードの読解が難しいため、この修正により可読性が増す。

また改修前はエラーメッセージオブジェクトのキーと、ヘルプページのキー名、そしてswitchのキーの全てが食い違っており、見ていて混乱する内容だったが、ヘルプページのURLをべた書きするように変えたため、混乱する要素を減らせている。

まとめ

コードの記述量を減らし、長大なコードを別々のファイルに分けることや、過度に分割しすぎないことによって可読性が向上し、コード量が減ったため保守コストも削減できたと考えている。

半面、静的解析のコストは上がっているが、静的解析のコスト増よりも可読性がよく、保守コストが低いコードのほうが効率的な開発に寄与するだろう。

また静的な値を取得するためにMapを使い、TypeScriptを利用しているため、理論上undefinedが返ってこないのに、それを期待するロジックを削除することで、コードを読んだ人が混乱する可能性も削減している。

型安全で見通しの良いエラーハンドリング

この項目ではエラーメッセージを返すサーバー側のエラーハンドリングについて記述する。

元コード

try {
  // API呼び出しなど
} catch (error) {
  if (error instanceof ApolloError) {
    if (error.graphQLErrors[0]?.extensions?.ERROR_KBN === 'ERR_XYZ') {
      // エラー処理
    } else if ...
    ...
  }

  if (error instanceof SomeError) {
    ...
  }
  ...
}

このコードの問題点

• エラーハンドリングが長々とべた書きされていて見通しが悪い
• 配列の添え字が直に指定されているが意図が読み取れない
• 例外処理なのにOptional chainingが多く確実な例外処理に支障がある（例外処理で例外が発生するリスクは最小限にする必要がある）
• extensions配下の型情報が暗黙的でわからない

  graphQLErrors: ReadonlyArray<{
   extensions: {
       [attributeName: string]: unknown;
   }
  }>
  

改善後コード

const handleApolloError = (err: ApolloError) => {
  if (err instanceof ApolloError) {
    // graphQLErrorsが空であれば例外をスローし、そうでなければgraphQLErrorsを返す
    const gqlErrors = parseGQLErrors(err.graphQLErrors);
    // graphQLErrorsの中身をパースし、意味のある型を付けて返す
    const extensions = parseThisFunctionErrors(gqlErrors);
    if (extensions.ERROR_KBN === 'ERR_XYZ') {
      // エラー処理
    } else if ...
  }
}

try {
  // API呼び出しなど
} catch (error) {
  if (error instanceof ApolloError) {
    handleApolloError(error);
  } else if (error instanceof SomeError) {
    ...
  }
  ...
}

べた書きされているとコードの見通しが悪いため、catch句の中では例外種別ごとにハンドリングする処理に飛ばし、そっちで処理できるようにする。

error.graphQLErrorsは要素が0の場合があるため、parseGQLErrors()のような共通関数を作り、要素があれば中身を返す、なければ例外をスローして、より上位の処理に飛ばすなどの処理を共通的に行うようにする。この仕組みを共通化することで、この要素に対する処理の一貫性を持たせることができる。

またgraphQLErrorsの詳細については処理によって内容が異なるであろうことから、ドメインごとにparseThisFunctionErrors()のような関数を作り、その中で適宜データを整形するのが望ましいだろう。

そうして結果的に意味のある型情報を持ったextensions、あるいは適当な結果情報をハンドリングすることで、型安全かつ、責務が別れ、疎結合な実装に寄与する。

この形式であればhandleApolloError()は必要に応じて別ファイルに切り出し単体テストを書くこともできるし、このままでもあっても規模が小さければ十分テスト可能だろう。

なお、parseGQLErrors()やparseThisFunctionErrors()の具体的な内容については今回は省略する。