綴Photo の勉強ノートに戻る
開発記録2026.05.14

コードレビューの指摘を、あえて途中で止める — マジックストリング対策の3段階

綴Photo
DartDriftenumコードレビュー設計判断綴Photo

綴Photo のコードレビューで、「マジックストリング」という指摘を受けた。

指摘自体は前から知っていたつもりだったけど、今回いちばん勉強になったのは 「指摘されたこと＝全部やるべき、ではない」 という判断の方だった。マジックストリングは段階的に直せます。そして、どこで止めるかは指摘とは別の判断になります。

その「段階」と「止めどき」を、AI（Claude Code）と対話しながら整理した記録を残す。

この記事の前提: 指摘は AI、概念整理も AI との対話

正直に書いておくと、マジックストリングを指摘したのは Codex（第三者レビュー用に回している別の AI）で、「文字列・定数・enum の3段階」を整理してくれたのは Claude Code との対話だった。

途中で「つまりこういうこと?」と自分なりにまとめ直したら、3点ほど認識がズレていて訂正された。その訂正も含めて、進行形の学習ログとして書く。

直し方は3段階ある

定数はソース。比較はメモリ。月の状態は端末 SQLite の PeriodVisitStatus に残る。段階1 生の文字列visited を直書きタイポは実行時まで不明型は String最悪段階2 定数PeriodVisitStatusValue名前のタイポは書く瞬間型はまだ Stringいまここ段階3 本物の enumPeriodVisitStatusEnum2値以外は書けない型そのものが変わるやらなかった指摘は直せ。実害のタイポ事故は段階2で潰れる。段階3は型の強制。

段階2: 定数にする（綴Photo は既にここまで来ていた）

dart
abstract class PeriodVisitStatusValue {
  static const visited = 'visited';
  static const completed = 'completed';
}
 
// 使う側
if (status == PeriodVisitStatusValue.completed) { ... }

PeriodVisitStatusValue.completed のように 名前付きの定数 にしておくと、PeriodVisitStatusValue.completd とタイポした瞬間に「そんな定数はない」とコンパイラ（書いたコードを実行できる形に変換する前に、文法や名前の取り違えを下読みしてくれる校正係）が弾いてくれます。冒頭の Callout で書いた「書いてる時点では誰も言ってくれない」の "誰か" が、定数化によってようやく登場します。

実は綴Photo のコードは、レビュー時点で既にここまで来ていた。コードを grep してみたら、生の 'visited' / 'completed' はもう残っていなくて、全部この定数経由で書かれていた。つまり 「マジックストリングを直せ」という指摘の実害部分（タイポ事故）は、すでに潰れていた。

段階3: 本物の enum にする

ここから先がレビューの「残り」だった。

dart
enum PeriodVisitStatusEnum { visited, completed }

段階2では status の型は依然 String で、内部で status を String のまま受け取る関数（綴Photo だと _upsertStatus(int year, int month, String status)）は技術的には 'バナナ' でも受け取れてしまいます。

おまけに switch (status) を書いたとき、全パターンを網羅しているかコンパイラがチェックしてくれます。将来 archived のような3つ目を足したら、対応漏れの switch を全部指摘してくれます。

自分なりにこう理解した（そして3点ズレていた）

ここまで聞いて、自分の言葉でこうまとめ直した。

文字列で判定してた → タイポすると条件が一致しなくなる → 変数として定義してタイポを無くした（ここまで完了）。enum という型を使えばリスト保管的になって応用が利く。でもそうすると DB が複雑になる。種類が多い時に検討すればいい。

大筋は合っていたけど、3点ズレていて訂正された。この訂正が地味に勉強になったので残す。

ズレ1: 「変数」じゃなくて「定数」 値が変わらないから「定数」。変数（後で書き換わる入れ物）だとタイポ防止の意味が薄れます。細かいけど、コンパイラが「その名前を知ってるか」で守ってくれている、という仕組みの理解に関わります。

ズレ2: enum は「リスト保管」「応用が利く」ではありません enum の核心は「決まった選択肢を1つの型に固める」こと。リスト（中身が増減する入れ物）とは別物だった。価値は「型システムが不正値を締め出す」「switch の網羅チェック」の2点で、"応用" という曖昧な言葉で捉えていたのがズレていた。

ズレ3: 「DB が複雑になる」じゃなくて「変換層が1枚増える」

保存先は端末 SQLite の PeriodVisitStatus.status。段階3でも TEXT。SQLite に enum 型はない。いま（段階2）コード 定数 StringPeriodVisitStatusValueソースとメモリ同じ文字列端末 SQLitePeriodVisitStatus.statusTEXT 閉じても残る段階3にするとコード enumPeriodVisitStatusEnumソースとメモリTypeConverter翻訳の交差端末 SQLitePeriodVisitStatus.status同じ TEXT。箱は増えないズレていた理解は DB が複雑になる。正確には交差が1本増えるだけ。

自分のまとめは8割合っていて2割ズレていた。ズレを潰しておかないと、次に似た判断をするとき間違えます。

で、段階3はやらないことにした

ここが今回いちばん残したい判断。

段階3（enum + TypeConverter）には、ちゃんとメリットがあります。型で不正値を完全に締め出せるし、switch の網羅チェックも効きます。でも、やめた。理由はコスト対効果。

  • 段階2（定数化）で、実害のあるリスク = タイポ事故はもう潰れています
  • 段階3で増える「型強制」「switch 網羅チェック」は、値が2つ・使用箇所も数ファイルで・将来増える気配もない この状況だと効果が薄いです
  • 一方コストは、Drift の自動生成コードを build_runner で作り直して、status を String 前提で扱っていた全箇所を enum 型に直して、それが噛み合うか検証する、という中規模の作業

「軽い掃除」のつもりで着手して、ふたを開けたら中規模だった。そして、その中規模をやり切っても得られる安全性の上乗せは薄いです。なので 「指摘は受けたが、ここでは段階2で十分」 と判断して、進捗メモには「status の値が増えたら enum 化を再検討」とだけ書いて閉じた。

段階3が「やる価値あり」になる条件

逆に、こういう時は enum + TypeConverter までやる価値が出る、という整理もできた。

  • enum の値が多いです（5個も10個もある）
  • 使用箇所が多くて、あちこちで switch 分岐しています
  • 値が将来増えそうです（増えた時の対応漏れが怖いです）
  • DB への不正値混入が現実的なリスク

PeriodVisitStatus はどれにも当てはまらなかった。だから止めた。

学んだこと

  • マジックストリング対策には段階があります。一気に最後までやるのが「正しい」わけじゃありません
  • コードレビューの指摘は「方向」であって「ゴール」ではありません。「マジックストリングを直せ」の実害部分は段階2で潰れていて、残りは「やってもいいが、ここでは要らない」だった。指摘を全部消化しようとすると、効果の薄い作業に時間を吸われます
  • 「軽い掃除」と思って着手したら中規模だった、はよく起きます。着手してスコープが見えた時点で「これは今やる価値があるか」を判断し直していいです
  • 非エンジニアが AI に実装を任せていると、概念の理解は「8割合って2割ズレてる」くらいの精度になりがちです。自分なりにまとめ直して、ズレを訂正してもらう プロセスを挟むと、次の判断の精度が上がります

おわりに

今回いちばん効いたのは、技術的な知識そのものより「指摘を全部やらなくていい」という感覚だった。

レビューで指摘がたくさん出ると、つい全部消化したくなります。でも、指摘ごとに「実害はどこまで残っているか」「直しきるコストはいくらか」を見ると、段階2で止めていいもの・段階3まで必要なものが分かれます。その仕分けができると、限られた時間を効果の大きいところに使えます。

この記事は2026年5月時点の自分の理解で書いています。仕様や認識が変わってる可能性があります。