Common Lisp で
「リファクタリング」を考える

  • リファクタリングでは「今の次の問題」に集中して解くことが、読みやすく変更しやすいコードを保つ判断基準になります。
  • Martin Fowlerのリファクタリング原則の考え方で、Common Lispのログ集計処理を分解してみました。
  • JavaはクラスやPrivateメソッドへの移動が必要な場面でも、Lispでは関数やマクロを切り出すだけで済むことが多いです。
  • データ表現も、シンプルなplistから始めてdefstructへ段階的に移行するなど、問題の複雑さに合わせて選べるのもLispの特徴です。

関連記事

1. Lisp での「リファクタリング」

複雑なコードを改善する考え方に、「リファクタリング」があります。

「リファクタリング」という言葉は、コードを書き直すことと混同されがちですが、バグを直すことでも機能を追加することでもない。変更しやすさと読みやすさを上げるための、ひとまとまりの小さな改善です。

Martin Fowler は、『Refactoring: Improving the Design of Existing Code』の中で「外部的な振る舞いを変えずに内部構造を改善し、変更しやすさを高める営みである」と定義しました1

初版では、Javaコードを題材に、クラス、継承、メソッド移動の話が中心でしたが、このFowler の原則は Common Lisp にも当てはまります。

ただし、選択する言語によって同じコードの「匂い」に対して、具体的な解説方法には違いがあります。

Common Lisp でリファクタリングを考える Java 静的型付け・クラス中心 「どのクラスに置くか」を問う ボイラープレートが多い メソッド移動が必要になる Common Lisp 関数はクラスに属さない 関数の粒度だけを問う マクロで構文を抽象化 データ表現の選択肢が豊富 VS 外部的な振る舞いを変えず、 内部構造を変更しやすく改善すること

Java は静的型付けとクラス中心の言語設計です。
そのため、責務の分離は「どのクラスにこのメソッドを置くか」という問いになります。
また、Javaコードには、書かなければならない定型的な記述(ボイラープレート)が多いぶん、コードを改善したくなる必然性が多いです。

一方、Common Lisp ではやや事情が異なります。
端的に言えば、Lisp系言語は「自由」です。

関数はクラスに属さない独立した単位で、線形リストによるデータ表現の選択肢も複数あります。
マクロで構文パターンそのものを抽象化できます。

1.1. アクセスログを解析する複雑なコード

Common Lispでのリファクタリングを具体的なコードから見ていきましょう。

Extract Function — 関数の粒度を問う Before summarize-log パース エラー判定 集計 出力 抽出 After parse-log-line 1行 → plist error-entry-p status ≥ 500 average-ms 合計 ÷ 件数 summarize-log 組み合わせるだけ 最初は labels で局所化して試す 他で使うなら defun へ引き上げる

アクセスログの1行を受け取り、エラー件数と平均レスポンスタイムを集計して出力する処理を考えます。

GET  /api/users  200  42
POST /api/orders 500  130
GET  /api/items  200  18
GET  /api/users  404  55
POST /api/login  200  98
GET  /api/orders 500  210

このアクセスログに対して、エラー数と平均レスポンスタイムは

errors: 2  avg-ms: 92Code language: HTTP (http)

各行を lines で与えて、処理する関数を作ってみます。

(defun summarize-log (lines)
  (let ((error-count 0)
        (total-ms 0)
        (total-count 0))
    (dolist (line lines)
      (let* ((parts    (cl-ppcre:split "\\s+" line))
             (status   (parse-integer (nth 1 parts) :junk-allowed t))
             (ms       (parse-integer (nth 2 parts) :junk-allowed t)))
        (when (and status ms)
          (incf total-count)
          (incf total-ms ms)
          (when (>= status 500)
            (incf error-count)))))
    (format t "errors: ~A  avg-ms: ~A~%"
            error-count
            (if (zerop total-count) 0
                (round (/ total-ms total-count))))))Code language: Lisp (lisp)

一つの関数にパース、集計、エラー判定、出力が混在していて、Fowler ならここに「長い関数」「責務の混在」という code smell を見つけるはずです2

1.2. 基本のリファクタリング

Common Lisp のリファクタリングでは、内部の処理を素直に3つの関数に分解できます。

(defun parse-log-line (line)
  (let* ((parts  (cl-ppcre:split "\\s+" line))
         (status (parse-integer (nth 1 parts) :junk-allowed t))
         (ms     (parse-integer (nth 2 parts) :junk-allowed t)))
    (when (and status ms)
      (list :status status :ms ms))))

(defun error-entry-p (entry)
  (>= (getf entry :status) 500))

(defun average-ms (entries)
  (if (null entries) 0
      (round (/ (reduce #'+ entries :key (lambda (e) (getf e :ms)))
                (length entries)))))Code language: Lisp (lisp)

これらの3つの関数を組み合わせると、summarize-log は、ログからエントリーをパースして、エラーを集計して、出力する、というロジックをそのまま書くことができます。

(defun summarize-log (lines)
  (let* ((entries      (remove nil (mapcar #'parse-log-line lines)))
         (error-count  (count-if #'error-entry-p entries))
         (avg          (average-ms entries)))
    (format t "errors: ~A  avg-ms: ~A~%" error-count avg)))Code language: Lisp (lisp)

Java と違うのは、この分解に「どのクラスに置くか」という問いが一切ないことです。
これが Lisp 的な設計の自由さの一つです。

1.3. 自動テストが変更の安全網になる

ただし、このようなコードの書き換えを一足飛びにしたわけではありません。

リファクタリングの前提はテストです。
Fowler は「テストなしのリファクタリングはリファクタリングではない」と言っています。

「リファクタリング」と「書き直し」の違いは、小さな変更を積み重ね、そのたびにテストが通ることを確認しながら進むこと。
一度に大きく書き換えると、どこで壊れたかわからなくなります。

Fowler は「二つの帽子」という比喩を使います。

  • 機能追加中はテストを増やして新しい振る舞いを足す(赤の帽子)。
  • リファクタリング中はテストを変えずに内部だけを整える(緑の帽子)。
    ついリファクタリング中に機能追加もしたくなりますが、同時にやろうとするとコードが動かなくなり、どこを直せばよいかわからなくなってしまいます。

Common Lisp には Parachute というテストフレームワークがあり、関数の隣にテストを置いて、ファイルをコンパイルするたびに自動実行できます。
C-c C-k(SLIME でファイル全体をコンパイルしてロードするショートカット)のたびにテストが走る仕組みの作り方は別記事で詳しく説明しています3

リファクタリング前の summarize-log にテストを書くとこうなります。

(dev-test-support:dev-test summarize-log-test
  (let ((lines '("GET /api/users 200 42"
                 "POST /api/orders 500 130"
                 "GET /api/items 200 18")))
    ;; error 1件、avg-ms は (42+130+18)/3 = 63
    (parachute:is = 1  (count-if #'error-entry-p
                                 (remove nil (mapcar #'parse-log-line lines))))
    (parachute:is = 63 (average-ms
                         (remove nil (mapcar #'parse-log-line lines))))))Code language: Lisp (lisp)

テストが通っている状態で関数を分解していけば、振る舞いが変わった瞬間に検知できます。

1.4. 「準備リファクタリング」というタイミング

では、いつリファクタリングをすればよいのでしょうか。
それは、新しい機能を追加する前です。

Fowler は「準備リファクタリング」と呼ばれるタイミングを推奨しています。
機能追加前に、その機能を追加しやすい形に整えておく。
そうすれば、機能追加は速くなります。

どの段階でリファクタリングを止めるべきか、という問題はまた後で考えます。

2. Extract Functionの手順(局所→トップレベル)

「リファクタリング」の手法の中でもっともよく使われるのは、Extract Function(関数抽出)です4
つまり、長い処理を意味のある名前の小さな関数に分けることです。

Common Lisp には、局所関数の仕組みもあり、まだトップレベル関数にするほどかどうかわからない段階でも、関数を切り出して試せます5

summarize-logのリファクタリングでは、関数を分割する前に、最初のステップとして labels で切り出しました。

(defun summarize-log (lines)
  (labels ((parse (line)
             (let ((parts (cl-ppcre:split "\\s+" line)))
               (let ((status (parse-integer (nth 1 parts) :junk-allowed t))
                     (ms     (parse-integer (nth 2 parts) :junk-allowed t)))
                 (when (and status ms)
                   (list :status status :ms ms)))))
           (error-p (entry)
             (>= (getf entry :status) 500))
           (avg-ms (entries)
             (if (null entries) 0
                 (round (/ (reduce #'+ entries :key (lambda (e) (getf e :ms)))
                           (length entries))))))
    (let* ((entries     (remove nil (mapcar #'parse lines)))
           (error-count (count-if #'error-p entries))
           (avg         (avg-ms entries)))
      (format t "errors: ~A  avg-ms: ~A~%" error-count avg))))Code language: Lisp (lisp)

labels で切り出すと 関数内部の見通しが少し改善されます。
parse は別のログフォーマット処理でも使いそうだ」と判断したらトップレベルへ引き上げ、「この関数にしか使わない」なら labels に留めておくのが局所性の観点から自然です6

Java では関数抽出に伴って「このメソッドを private にするかどうか」「どのクラスに属して公開するか」を考える必要がありますが、Common Lisp では「トップレベルに出すかどうか」という設計判断に相当するわけです。

2.1. 抽出する関数に良い名前をつける

ところで、「リファクタリング」では名前が重要です。

意外と混同しやすいのは、関数の名前は「何をするか」を表すべきで、「どうやってするか」は本体に任せます7

;; 【よい例】
(defun parse-log-line (line) ...)

;; 【よくない例】
(defun split-and-pick-fields (line) ...)Code language: Lisp (lisp)

「何をするか」を関数名にすると、その関数が持つ責任が明確になります。
テストを書くときも、テスト対象の意図が名前から読めるため書きやすくなります。

2.2. 処理の重複には高階関数

重複したコードがバラバラにあると、変更が必要なときにもれなく変更するのが大変です。

Fowler は重複排除を一貫して重視していて、Extract Function はその第一歩です。
重複の種類によって解決の道具が変わります。

重複排除:高階関数、そしてマクロ 処理の重複 → 高階関数 count-5xx count-4xx count-2xx count-entries-matching entries + predicate(λ) 関数を値として渡せる 構文の重複 → マクロ with-open-file → loop → 処理A with-open-file → loop → 処理B with-log-lines マクロ 骨格を一箇所に集約 継承なしで骨格と差分を分離 with-open-file と同じ発想 まず関数 → 高階関数 → 構文ならマクロ

たとえば、ログの種類が増えると、同じ集計パターンが繰り返されることがあります。
エントリーのstatusの値を元に個数を調べています。

(defun count-5xx (entries)
  (loop for e in entries count (>= (getf e :status) 500)))

(defun count-4xx (entries)
  (loop for e in entries count (and (>= (getf e :status) 400)
                                    (< (getf e :status) 500))))

(defun count-2xx (entries)
  (loop for e in entries count (and (>= (getf e :status) 200)
                                    (< (getf e :status) 300))))Code language: Lisp (lisp)

loop for e in entries count <条件> というパターンが繰り返されています。
このようなパターンは、高階関数で統一できます。
高階関数(higher-order function)は、関数そのものを引数や返り値として扱う関数のことです。

count-entries-matching関数は、predicateにラムダ式を与えて、集計条件を変えられる設計になっています。

(defun count-entries-matching (entries predicate)
  (loop for e in entries count (funcall predicate e)))

(count-entries-matching entries 
                        (lambda (e) (>= (getf e :status) 500)))
(count-entries-matching entries 
                        (lambda (e) (< (getf e :status) 400)))Code language: Lisp (lisp)

funcall は関数オブジェクトを呼び出すための関数で、loopが共通化され変更点が count-entries-matching の呼び出しに集まります。

2.3. 構文パターンの重複にはマクロ

ログ処理では、ファイルを開いて一行ずつ行を読む、という骨格があちこちに現れることがあります。

(defun analyze-access-log (path)
  (with-open-file (stream path)
    (let ((lines (loop for line = (read-line stream nil)
                       while line collect line)))
      (summarize-log lines))))

(defun analyze-error-log (path)
  (with-open-file (stream path)
    (let ((lines (loop for line = (read-line stream nil)
                       while line collect line)))
      (count-entries-matching
        (remove nil (mapcar #'parse-log-line lines))
        #'error-entry-p))))Code language: Lisp (lisp)

ファイルを読んで行リストに変換する部分が繰り返されています。

高階関数でも書けますが、with-xxx という形のマクロにすると呼び出し側がより自然に読めます8

(defmacro with-log-lines ((var path) &body body)
  `(with-open-file (,var ,path)
     (let ((,var (loop for line = (read-line ,var nil)
                       while line collect line)))
       ,@body)))Code language: Lisp (lisp)

with-log-linesマクロを使うと、すぐにlinesで各行を集計できます。

(defun analyze-access-log (path)
  (with-log-lines (lines path)
    (summarize-log lines)))

(defun analyze-error-log (path)
  (with-log-lines (lines path)
    (count-entries-matching
      (remove nil (mapcar #'parse-log-line lines))
      #'error-entry-p)))Code language: Lisp (lisp)

このマクロは、Fowler の言うテンプレートメソッドパターンに相当します。
しかし、Lispでは継承を使わずに骨格と差分を分離できています。
with-open-filewith-slots といった Common Lisp 標準の with-xxx マクロと同じ発想です9

ただし、マクロは強力ですが、何が返ってくるのか理解コストが高い。
無理にマクロ化すると、かえってコードの意味が見えにくくなります。

重複排除では、まずは関数抽出。
似たようなパターンの関数が増えてきたら、高階関数やマクロを検討します。

3. Introduce Parameter Object(データ表現)

次は、「Introduce Parameter Object」。

Fowler の Introduce Parameter Object は、関数の引数が増えてきたときに関連するデータをグループ化する手法です。

;; フィールドが増えてきた
(defun analyze-entry (method path status ms referer user-agent)
  ...)Code language: Lisp (lisp)

Common Lisp では、同じ「引数群をまとめる」問題でも、

  • plist:キーと値を交互に並べたリスト、
  • defstruct による構造体、
  • alist:キーと値のペアのリスト)など、

問題の複雑さに合わせて選べます。

データ表現の見直し plist まず試す 定義不要・追加が楽 スペルミスは実行時まで不明 安定 defstruct フィールド固定後 アクセサ自動生成 コンパイル時エラー検知 設定化 alist 変更頻度高い ルール・設定 defparameter で管理 条件分岐 → データテーブルへ cond で書くと ルール追加のたびに関数変更 *status-level-rules* に書くと リストに追加するだけで完結

3.1. まず plist で試す

ログエントリのフィールドが増えてきたとき、plist で包むのが最も軽い選択です。
plist(property list) は、キーと値を交互に並べたリストです。

;; plist でまとめて渡す
(defun analyze-entry (entry)
  (let ((method     (getf entry :method))
        (path       (getf entry :path))
        (status     (getf entry :status))
        (ms         (getf entry :ms))
        (referer    (getf entry :referer "-"))
        (user-agent (getf entry :user-agent "-")))
    ...))Code language: Lisp (lisp)

plistは、リスト構造の自然な応用なので、構造を定義する手間がなく、フィールドの追加も容易です

3.2. 構造が安定したら defstruct へ

ただし、plistには、柔軟さゆえの欠点もあります

スペルミスがあっても実行時まで気づきにくいということ10

そこで、フィールドの種類が固まってきたら defstruct で構造体にします。

(defstruct log-entry
  method
  path
  status
  ms
  (referer    "-")
  (user-agent "-"))

;; アクセサ関数で値を取る
(log-entry-status entry)
(log-entry-ms     entry)Code language: Lisp (lisp)

defstruct はアクセサ関数とコンストラクタ(make-log-entry)を自動生成します。存在しないフィールドへのアクセスはコンパイル時にエラーになるため、plist より安全です11

3.3. 条件分岐はデータテーブルに変える

データ表現の見直しは、引数のまとめ方だけにとどまりません。

条件分岐そのものをデータとして表現し直せる場面があります。
ログレベルの判定がその典型例です。

(defun log-level (status)
  (cond ((>= status 500) :error)
        ((>= status 400) :warn)
        ((>= status 300) :info)
        (t               :ok)))Code language: Lisp (lisp)

ステータスコード範囲とログレベルの対応が条件分岐に書かれています。
しかし、この対応は本来データです。

(defparameter *status-level-rules*
  '((500 . :error)
    (400 . :warn)
    (300 . :info)))

(defun log-level (status)
  (or (cdr (find-if (lambda (rule) (>= status (car rule)))
                    *status-level-rules*))
      :ok))Code language: Lisp (lisp)

このように判定ルールをデータで管理すれば、ルールが増えても *status-level-rules* に追加するだけで、関数本体には触れなくて済みます。

Fowler の言う「変更点を一箇所に集める」がデータの側で実現されています。
ルールを設定ファイルから読み込む形への移行も、この構造なら自然に行えます。

3.4. plist、defstruct、alist の使い分け

どのデータ構造を選択するかは、自明ではありません。

そんなときは、まずは、plist で始めて、構造が見えてきたら defstruct に切り替える、という考え方でよいです12

状況選択
フィールドがまだ固まっていないplist
フィールドが安定し、アクセスの安全性がほしいdefstruct
キーと値の対応を検索・追加したいalist
変更頻度が高いルールや設定パラメータ + alist

リファクタリングのプロセスでは、段階的に進めるものです。
はじめから「正解の設計」は見つからない前提で、進めて大丈夫です。

4. リファクタリング後のコード全体

ここまで各節でバラバラに示してきたコードを一本にまとめます。
これが今回のリファクタリングの到達点です。

;;; ログエントリのデータ表現
(defstruct log-entry
  method
  path
  status
  ms
  (referer    "-")
  (user-agent "-"))

;;; ステータスコードとログレベルの対応(変更頻度が高いのでデータとして持つ)
(defparameter *status-level-rules*
  '((500 . :error)
    (400 . :warn)
    (300 . :info)))

;;; パース
(defun parse-log-line (line)
  (let* ((parts  (cl-ppcre:split "\\s+" line))
         (method (nth 0 parts))
         (path   (nth 1 parts))
         (status (parse-integer (nth 2 parts) :junk-allowed t))
         (ms     (parse-integer (nth 3 parts) :junk-allowed t)))
    (when (and status ms)
      (make-log-entry :method method :path path
                      :status status :ms ms))))

;;; 判定
(defun error-entry-p (entry)
  (>= (log-entry-status entry) 500))

(defun log-level (status)
  (or (cdr (find-if (lambda (rule) (>= status (car rule)))
                    *status-level-rules*))
      :ok))

;;; 集計
(defun average-ms (entries)
  (if (null entries) 0
      (round (/ (reduce #'+ entries :key #'log-entry-ms)
                (length entries)))))

(defun count-entries-matching (entries predicate)
  (count-if predicate entries))

;;; 出力
(defun summarize-log (lines)
  (let* ((entries     (remove nil (mapcar #'parse-log-line lines)))
         (error-count (count-entries-matching entries #'error-entry-p))
         (avg         (average-ms entries)))
    (format t "errors: ~A  avg-ms: ~A~%" error-count avg)))

;;; ファイル読み込みの骨格
(defmacro with-log-lines ((var path) &body body)
  `(with-open-file (,var ,path)
     (let ((,var (loop for line = (read-line ,var nil)
                       while line collect line)))
       ,@body)))

;;; エントリポイント
(defun analyze-log (path)
  (with-log-lines (lines path)
    (summarize-log lines)))Code language: Lisp (lisp)

最初の summarize-log と比べると、だいぶ長いコードになりましたね。
どう「読みやすく」なったのでしょうか。

1つ目は、構造です。
コード全体を読むと長く見えますが、「全体は読まない」という発想に切り替えます。
最初に注目すべきは summarize-log 関数だけ。

(defun summarize-log (lines)
  (let* ((entries     (remove nil (mapcar #'parse-log-line lines)))
         (error-count (count-entries-matching entries #'error-entry-p))
         (avg         (average-ms entries)))
    (format t "errors: ~A  avg-ms: ~A~%" error-count avg)))Code language: Lisp (lisp)

ここだけ見れば短い、と言えます。
もともと、コードは部品の組み合わせです。
読むときには、必要な部品だけを確認していけば、全体を読まずに済みます。

ライブラリ関数をすべて読まずとも、コードから処理は読み取れますよね。
自分のコードも「ライブラリ」のように扱えれば、読む範囲はぐんと減ります。

次は、テストです。
最初の版では一つの関数に全部が詰まっていたため、パースだけを検証する手段がありませんでした。
分解することで、parse-log-lineerror-entry-paverage-ms はそれぞれ単独で呼び出せるようになりました。

そして、メインは機能の変更しやすさです。
最初の版では判定ロジックが集計ループの中に埋まっていたので、コード全体から変更箇所をもれなく探し出すところから始める必要がありました。
これが神経を使います。
リファクタリング後は、ログフォーマットを変えたいなら parse-log-line だけ、ステータス判定のルールを変えたいなら *status-level-rules* だけ触れば済みます。

ある意味、リファクタリングでは、「コード全体から変更箇所をもれなく探し出す」ということを、あらかじめ抽象化しているのです。

とはいえ、parse-log-line には、まだ改善の余地もあります。
フィールドの位置を nth 0nth 1nth 2nth 3 でハードコードしているので、ログフォーマットが変わったときには、この番号を変える必要があります。
より柔軟性を持たせるためには、ログフォーマットをデータとして外に出すか、フィールド名で参照できる仕組みに変えるなどの工夫ができます。

ただし、リファクタリングでは「完成した状態」はありません。
常に次の要件変更に備えて、今の問題を一段階小さくしておく、という繰り返しなので、どこかで切り上げる必要もあります。

4.1. リファクタリングから見る Cmmon Lisp の特徴

Common Lisp は、ある意味 Javaとは対局の言語です。
そのため、Javaのリファクタリング手法に比べると、シンプルです。

  • Java でメソッド移動が必要になる場面が、Lisp では関数の粒度を変えるだけで済む
  • Java でテンプレートメソッドパターンや継承が必要だった「骨格と差分の分離」を、with-xxx マクロ一つで表現できる
  • Java では「データをまとめる」イコール「クラスを作る」になりやすいが、設計の初期段階でクラス設計を強いられない
  • Java では振る舞いはオブジェクトの所有物なので条件分岐の置き換えにポリモーフィズムが必要だが、Lisp では条件分岐をデータテーブルに変えるか、高階関数に切り出すだけで足りる

あるいは、リファクタリング手法を意識せずとも、自然と変更しやすいコードになります。

code smellJava での典型的な解法Lisp での解法変わる理由
長い関数・責務の混在private メソッドに分割、クラスへ移動labels で局所化 → defun へ引き上げ関数がクラスに属さない
重複コード(処理)共通メソッドへ抽出、継承で共有高階関数(mapcarcount-if など)関数が第一級の値として扱える
重複コード(構文)テンプレートメソッドパターン、継承with-xxx マクロマクロが構文を扱える
長い引数リストParameter Object クラスを定義plist → defstruct へ段階的に移行データ表現の多様性
条件分岐の爆発ポリモーフィズム、Strategy パターンデータテーブル化(defparameter + alist)振る舞いをオブジェクトに閉じ込めなくていい
投機的一般化抽象クラス・インターフェースの先回り早すぎるマクロ化、DSL の乱立Lisp 版の同じ問題
変更の分散Shotgun Surgery → 責務をクラスに集約変更点をデータか関数一つに集めるどちらの言語でも同じ原則

ただし、Lispは柔軟に書けるため、エラーチェックは丁寧にする必要があります。
Java はクラス・型・アクセス修飾子が「ある程度の構造」を強制します。
どこかで設計が崩れかけると、コンパイルエラーや型の不整合として表面化しやすい。
Lisp にはその強制が弱いからです。

5. リファクタリング原理主義と「投機的一般化」

「リファクタリング」は原理主義的に適用すると、最初のフラットなコードより読みにくくなります。

Fowler の『Refactoring』は過剰なリファクタリングについても「Speculative Generality(投機的一般化)」という code smell として取り上げています13

投機的一般化 と やめどき Speculative Generality ① format-summary(3行) 「変わるかも」 ② *summary-format* を外部化 「汎用化できる」 ③ render-fields を抽出 ④ マクロ+データ定義 30行超 リファクタリングをやめるとき 今の問題が解決した 変更箇所が一箇所に集まった 関数が単独でテストできる 変わる見込みのない部分 一箇所にしか現れないパターン 使われない柔軟性のための構造

たとえば、重複を見つけるたびにマクロを書き、責務を感じるたびに抽象層を重ねる。
積み重なるとかえって読みにくくなってしまうのです。

コードで見てみましょう。
次の format-summary は、集計結果を文字列にフォーマットするだけの素直な関数です。

(defun format-summary (error-count avg-ms)
  (format nil "errors: ~A  avg-ms: ~A" error-count avg-ms))Code language: Lisp (lisp)

これを「出力フォーマットは変更される可能性がある」「フィールドの結合は再利用できるはずだ」と考えて、段階的に「改善」していくとどうなるか。

;; ① フォーマット文字列を外に出した(変更の局所化が目的)
(defparameter *summary-format* "errors: ~A  avg-ms: ~A")

(defun format-summary (error-count avg-ms)
  (format nil *summary-format* error-count avg-ms))Code language: Lisp (lisp)

ここまでなら許容範囲です。
フォーマット文字列を差し替えられるようになりました。

ところが「フィールドのペアを汎用化できる」と考えてさらに進めると、こうなります。

;; ② フィールドの結合を汎用化した(重複排除が目的)
(defun render-fields (fields)
  (format nil "~{~A: ~A~^  ~}" fields))

(defun format-summary (error-count avg-ms)
  (render-fields (list "errors" error-count "avg-ms" avg-ms)))Code language: Lisp (lisp)

render-fields は汎用的に見えます。
しかし ~{~A: ~A~^ ~} という format 指定子を読める人は限られます。

さらに「フィールド定義もデータにすべきだ」と進むと、こうなります。

;; ③ フィールド定義をデータ化した(データ駆動化が目的)
(defparameter *summary-fields*
  '((:key :error-count :label "errors")
    (:key :avg-ms      :label "avg-ms")))

(defmacro with-summary-context (bindings &body body)
  `(let ,(mapcar (lambda (b) (list (car b) (cadr b))) bindings)
     ,@body))

(defun format-summary (stats)
  (with-summary-context ((error-count (getf stats :error-count))
                         (avg-ms      (getf stats :avg-ms)))
    (render-fields
      (loop for field in *summary-fields*
            collect (getf field :label)
            collect (eval (getf field :key))))))Code language: Lisp (lisp)

ここまで来ると、format-summary が何をしているかを理解するために with-summary-contextrender-fields*summary-fields* の三つを追いかける必要が生まれました。
最初の3行が、30行以上の理解負荷に変わっています。

もちろん、各ステップで「正当なリファクタリングの理由」がありました。
しかし、変更される見込みのないフォーマット文字列を汎用化し、一箇所にしか現れないパターンをマクロ化し、使われない柔軟性のためにデータ構造を増やした。
問題は手法ではなく、「なぜ今これが必要か」を問わなかったことです。

Fowler 自身は「リファクタリングはそれ自体が目的ではない」と言っています。
コードを変更しやすく、読みやすくするための手段であって、きれいにすること自体がゴールではない。

5.1. リファクタリングを止める判断を振り返る

この記事のログ解析コードには、意図的に多めのリファクタリングを適用しました。
各手法を一通り示すためです。
実際の作業では、必要な変更だけを段階的に行い、「読みやすくなった」「変更の影響が一箇所に収まった」と感じた時点で止めます。

どこで止めるかに正解はなく、コードの変更頻度やチームの読解コストによって変わります。

以下に、それぞれの判断と「なぜそこで止めたか」を振り返ってみます。

現時点で変わりそうな箇所か

summarize-log をパース・判定・集計・出力に分けたのは、混在していると「どのステップで壊れたか」がテストで特定できず、変更箇所も探しにくかったからです。

ただし、format t の出力先を抽象化する、エラー判定の閾値をパラメータ化するといった方向には進んでいません。
それらが変わる見込みが現時点でなかったからです。

別の関数で使うことが確定したか?

parseerror-pavg-ms をまず labels に切り出したのは、この関数の外で使われるかどうかまだわからなかったからです。
labels のままで十分な場面もあります。
その後トップレベルに引き上げたのは、別の関数でも使うことが確定したからです。

最初からトップレベルに出すと、「この関数にしか使わない処理がグローバルに見えている」状態になります。

値で足りる問題か?

count-5xxcount-4xxcount-2xxcount-entries-matching に統一したのは、同じ構造が三つあり増え続けることが明らかだったからです。

マクロではなく高階関数を選んだのは、条件を「渡せる値」として扱えば十分で、構文の変換は不要だったからです。

別の関心事は混ぜない

骨格が二箇所に現れ、差分が処理の中身だけだったのでマクロにしました。
高階関数でも書けますが、(with-log-lines (lines path) ...) という形の方が「ファイルを読んで処理する」という意図が呼び出し側に素直に伝わります。

出力フォーマットの制御はここに入れませんでした。
集計と出力は別の関心事で、混ぜると「集計だけ返す」用途に対応できなくなるからです。

保持するデータの種類に十分か?

フィールドの種類が安定してきた段階で移行しました。
plist のままだとスペルミスが実行時まで気づかれません。

また、CLOS のクラスにはしませんでした。
ログエントリに振る舞いを持たせる必要がなく、データ保持だけなら defstruct で十分だからです。

条件はシンプルな対応関係か?

ステータスコードの範囲とログレベルの対応は「設定の知識」であってロジックではないので、データにしました。
cond に書くとルールを追加するたびに関数本体を変更しなければなりません。

ただし設定ファイルへの外部化はしませんでした。
起動時に差し替えたいという要求がまだないからです。
外部化はその要求が生まれてから対応すれば十分です。

5.2. 「一歩先の変更」に備える

判断に共通しているのは「今の問題を解くか、将来の問題を先回りするか」という問いです。

今の問題、つまりテストできない・変更箇所が散らばっている・重複が増え続けている、これらには対処しました。
しかし、将来の問題、つまりフォーマットが変わるかもしれない・外部設定が必要になるかもしれない、これらには手をつけていません。

Fowler の言葉を借りれば、リファクタリングは「次の変更」を迎えやすい形にしておく作業であって、「あらゆる未来の変更」に備える作業ではないからです。

  1. 『Refactoring: Improving the Design of Existing Code』は1999年に初版が出版され、2018年に第2版が刊行されました。第2版では例のコードが JavaScript に変更され、クラスを使わない関数型スタイルのリファクタリング例も追加されています。 – Refactoring (2nd ed.) – martinfowler.com
  2. code smell という用語は Kent Beck が考案し、Fowler の『Refactoring』初版(1999年)で広まりました。「表面に現れた兆候であり、より深い問題を示すことが多い」という定義で、バグそのものではなく設計上の問題を示すシグナルとして使われます。 – Code Smell – martinfowler.com
  3. eval-when (:load-toplevel)find-package :swank を組み合わせることで、SLIME 接続中のロード時だけテストが走り、本番ロードや CI では自動的にスキップされます。 – コードの修正で自動テストするには?(Parachute)
  4. 第1版では「Extract Method」という名称でしたが、第2版では「Extract Function」に改称されました。クラスに属さない関数形式のリファクタリングへの対応を反映した変更です。 – Refactoring (2nd ed.) – Goodreads
  5. fletlabels はどちらも局所関数を定義しますが、labels では定義した関数名がその定義本体の中でも参照できるため、再帰関数を書けます。flet では定義内で自身を呼び出せません。 – CLHS: Special Operator FLET, LABELS, MACROLET
  6. labels で定義した局所関数は、外側スコープの変数を閉包として参照できます。局所関数が引数を減らして書けるのもこのためです。 – Practical Common Lisp: The Special Operators
  7. Fowler は「コンピュータが理解できるコードは誰でも書ける。人間が理解できるコードを書けるのが優れたプログラマだ」と述べています。関数名はその「人間が理解できる」部分の中心に位置します。 – Refactoring: Improving the Design of Existing Code – InformIT
  8. unwind-protect は ANSI Common Lisp の特殊オペレータで、保護フォームの終了後(正常終了・非ローカル脱出のいずれでも)にクリーンアップフォームを必ず実行します。with-open-file はこれを使ってファイルを確実にクローズします。Java の try-finally に相当します。 – CLHS: Special Operator UNWIND-PROTECT
  9. SBCL の with-open-file は内部で unwind-protect を使い、例外が発生してもファイルを確実にクローズします。標準ライブラリの with-xxx マクロはこのパターンの典型例です。 – The Common Lisp Cookbook – Error and exception handling
  10. getf はキーの比較に eq を使います。そのためキーにはキーワードシンボル(:status など)を使うのが一般的です。文字列や数値をキーにすると eq で比較されるため、意図した動作にならない場合があります。 – Practical Common Lisp: Beyond Lists
  11. defstruct は構造体名に -p を付けた型判定述語(例:log-entry-p)と、copy- を付けたコピー関数(例:copy-log-entry)も自動生成します。 – CLHS: Macro DEFSTRUCT
  12. alist と plist はどちらも小規模なキーと値の対応に向いています。alist はエントリを前方に追加して後から取り消す(shadow)使い方ができるのに対し、plist は &key 引数リストと同じ形式のため apply で直接渡せるという利点があります。大規模なデータには hash-table を使うのが一般的です。 – The Common Lisp Cookbook – Data structures
  13. 「いつか使うかもしれない」という理由で複雑な仕組みを作ることへの警告です。 – Refactoring – martinfowler.com