準備的リファクタリングの例

2015年1月5日



リファクタリングがプログラミングのワークフローに適合する方法は様々です。便利な概念の一つに、準備的リファクタリングというものがあります。これは、新しい機能を追加する際に、既存のコードが、その機能を追加しやすい構造になっていないことに気づいた場合に実施するものです。そのため、まずコードを機能追加が容易になるような構造にリファクタリングします。ケント・ベックが簡潔に述べたように「変更を容易にし、それから容易な変更をする」のです。

最近のRuby Roguesのポッドキャストで、ジェシカ・カーが準備的リファクタリングについて素敵な比喩を語っていました。

まるで、東に100マイル進みたいのに、ただ単に森を歩き回るのではなく、まず高速道路まで北に20マイル進んでから、東に100マイル進むようなものです。そうすれば、まっすぐ進むよりも3倍の速さで進めます。人々がまっすぐ進めと急かしてくる時には、「ちょっと待って、地図を見て最短ルートを見つける必要がある」と言う必要があります。準備的リファクタリングは、そのためのものです。

-- ジェシカ・カー

私がこれまで出会った別の良い比喩は、壁を塗る際に、電気ソケット、ドア枠、幅木などにテープを貼ることです。テープを貼ることは塗装をすることではありませんが、最初にテープを貼ることに時間を費やすことで、塗装がより迅速かつ簡単になります。

一般的な説明や比喩はどれも素晴らしいものですが、例を示すのも良いでしょう。そして最近、私自身が共有する価値のある例に出会いました。

出発点

私の出版ツールチェーンには、ライブファイルからコードを記事に挿入する機能が含まれています。「ライブファイル」とは、コンパイルと実行が可能なコード、通常は教育用の(つまり、おもちゃのような)例のことです。ライブファイルからコードを読み込めることは、コピー&ペーストの問題を回避し、記事中のコードが実際にコンパイルされてテストに合格するコードであることを確信できるため、非常に役立ちます。私はコメント内のマーカーによってコードセクションをマークします。 [1]

現在追加している機能の1つは、これらのコードフラグメントの特定の部分を強調表示する機能です。この方法により、コードの1行または一部を取り出し、htmlでspan要素で囲むことで、CSSを使用して任意の方法で強調表示できます。この記事の後半で、私がこれを行っている例を見ることができます。これはリファクタリングについて議論する際に特に便利です。

ここで話しているプログラミングエピソードの開始時には、すでに特定の行、または特定の行内のコードのスパンを強調表示する機能がありました。3つ目の機能として、行の範囲を強調表示する機能を追加したかったのです。

記事のソースドキュメントでは、insertCode XML要素を使用してコードフラグメントを挿入したいことを示します。現在のハイライト機能では、ハイライトの束を定義できます。以下に例を示します。

<insertCode file = "Notification.java" fragment = "notification-with-error">
  <highlight line="add\(" span="new.*e\)"/>
  <highlight line="map"/>
</insertCode>

これはこのようなコードを強調表示します

public void addError(String message, Exception e) {
  errors.add(new Error(message, e));
}

public String errorMessage() {
  return errors.stream()
          .map(e -> e.message)
          .collect(Collectors.joining(", "));
}

insertCode要素には、ファイルパスと抽出したいフラグメントの名前の属性があります。次に、子要素でハイライトを指定できます。各ハイライトは、行を照合するために使用する正規表現を提供することで行を指定します。また、スパン属性(別の正規表現)を指定することもでき、その場合は、その正規表現に一致する行の部分のみが強調表示されます。スパンを指定しない場合、ハイライトは行全体に適用されます。

強調表示を行うコードを独自のクラスに配置しました。(ここでは気にする必要のない)別のコードがソースファイルからコードフラグメントを抽出し、強調表示が必要かどうかを確認します。必要な場合は、CodeHighlighterオブジェクトを作成して、強調表示を実行するように指示します。コードハイライターの呼び出しは次のようになります。

output << CodeHighlighter.new(insertCodeElement, codeFragment).call

これは、メソッドオブジェクトパターンを使用しており、オブジェクトを使用してファーストクラス関数を表現しています。その関数の引数を使用してオブジェクトを作成し、別のメソッドを呼び出して関数を実行し、結果を返し、その後メソッドオブジェクトをガベージコレクションに任せます。

ハイライターの実装は次のとおりです。

class CodeHighlighter
  def initialize insertCodeElement, fragment
    @data = insertCodeElement
    @fragment = fragment
  end
  def call
    @fragment.lines.map{|line| highlight_line line}.join
  end
  def highlight_line line
    highlights
      .select{|h| Regexp.new(h['line']).match(line)}
      .reduce(line){|acc, each| apply_markup acc, each}
  end
  def highlights
    @data.css('highlight')
  end
  def apply_markup line, element
    open = "<span class = 'highlight'>"
    close = "</span>"                 
    if element.key? 'span'
      r = Regexp.new(element['span'])
      m = r.match line
      m.pre_match + open + m[0] + close + m.post_match
    else
      open + line.chomp + close + "\n"
    end
  end
end

ツールチェーンのコードはRubyで、XMLを操作するためにNokogiriライブラリを使用しています。

ここでは、表示を混乱させるような方法で複数のハイライトを指定した場合など、エッジケースについてはあまり心配していません。結局のところ、これらの問題が発生した場合、私はどこに住んでいるかを知っています。これは、私が唯一のユーザーであるコードを書くことの贅沢さです。

ハイライターのテスト

ハイライターのテストは非常に簡単で、いくつかの入力を受け取り、いくつかの出力を発行する純粋な関数です。しかし、テストを書きやすくするために、テストクラスに少し仕掛けを施しました。まず、入力と出力のテキストの塊を別のファイルに保持します。これは次のようになります。

codeHighlighterHunks.txt…

  input
  
    private void validateDate(Notification note) {
      if (date == null) {
        note.addError("date is missing");
        return;
      }
  
      LocalDate parsedDate;
      try {
        parsedDate = LocalDate.parse(getDate());
      }
    } //end
  
  %% one-line
  
    private void validateDate(Notification note) {
      if (date == null) {
  <span class = 'highlight'>      note.addError("date is missing");</span>
        return;
      }
  
      LocalDate parsedDate;
      try {
        parsedDate = LocalDate.parse(getDate());
      }
    } //end
  
  %% one-span
  
    private void validateDate(Notification note) {
      if (date == null) {
        note.<span class = 'highlight'>addError</span>("date is missing");
        return;
      }
  
      LocalDate parsedDate;
      try {
        parsedDate = LocalDate.parse(getDate());
      }
    } //end
  …

ここには、%%で区切られた3つのテキストの塊があります。最初の塊は私の(最初の)入力文字列で、次の2つは1行の場合と1行内のスパンの場合の出力です。各塊にはキーがあり、これは行の%%に続くテキストです。その後、テスタークラスで塊に簡単にアクセスできます。

class CodeHighlighterTester…

  def hunks
    raw = File.read('test/codeHighlighterHunks.txt').split("\n%%")
    raw.map {|r| process_raw_hunk r}.to_h
  end
  def process_raw_hunk hunk
    lines = hunk.lines
    key = lines.first.strip
    value = lines
      .drop(1)
      .drop_while {|line| (/[^[:space:]]/ !~ line)}
      .join
    return [key, value]
  end

塊を簡単に抽出できるため、テストでそれらを参照できます。

class CodeHighlighterTester…

  def test_no_highlights
    assert_equal hunks['input'], with_highlights(form_element(""))
  end
  def test_one_line_highlight
    element = form_element "<highlight line = 'missing'/>"
    assert_equal hunks['one-line'], with_highlights(element)
  end
  def test_highlight_span
    element = form_element "<highlight line = 'missing' span = 'addError'/>"
    assert_equal hunks['one-span'], with_highlights(element)
  end
  def form_element s
    Nokogiri::XML("<insertCode>" + s + "</insertCode").root
  end  
  def with_highlights element, input = nil
    input ||= hunks['input']
    CodeHighlighter.new(element,input).call
  end

これに複数行の文字列またはhere docを使用することもできましたが、テキストの塊の方が扱いやすいと思います。

ハイライト範囲の追加

追加したかった新機能は、次のように行の範囲を強調表示することでした。

<insertCode file = "BookingRequest.java" fragment = "done">
  <highlight-range start-line = "missing" end-line = "return"/>
</insertCode>

開始行と終了行の属性は、範囲の最初と最後の行に一致する正規表現です。

最初に、新しいマークアップ動作のテストを追加し、それが失敗することを確認してから、スキップするようにマークしました。最初に、欲しい最終的な動作のテストを書くのが好きです。なぜなら、それが私にとって、望む結果が正確に何であるか、またAPIをどのように動作させたいかを明確にするからです。しかし、準備的リファクタリングを行うつもりなら、そのテストの失敗でテスト出力がごちゃごちゃになるのは嫌なので、一度失敗するのを見た後、作業中はスキップします。

class CodeHighlighterTester…

  def test_highlight_range
    skip
    e = '<highlight-range start-line = "(date == null)" end-line = "}"/>'
    assert_equal hunks['range'], with_highlights(form_element(e))    
  end

codeHighlighterHunks.txt…

  %% range
  
    private void validateDate(Notification note) {
  <span class = 'highlight'>    if (date == null) {
        note.addError("date is missing");
        return;
      }</span>
  
      LocalDate parsedDate;
      try {
        parsedDate = LocalDate.parse(getDate());
      }
    } //end

どのようにアプローチするかを考えていると、コードの強調表示を、提供されたテキストに対する一連の変換として扱うことができると判断し始めました。最初にハイライト範囲の変換を適用し、その後に既存のハイライトを適用します。この考えを頭の中からコードに移すことができます。

最初のステップは、callの本体全体に対してメソッド抽出を使用することです。

class CodeHighlighter…

  def call 
    apply_highlights @fragment.lines
  end
  def apply_highlights lines
    lines.map{|line| highlight_line line}.join
  end

ここで、ネストされた、何もしない関数、つまり変更せずに与えられたものを返す関数を導入します。

class CodeHighlighter…

  def call
    apply_highlights(apply_ranges(@fragment.lines))
  end
  def apply_ranges lines
    lines
  end

この単一のリファクタリングは、この記事全体の核心であり、単純なステップにまとめられています。このリファクタリングにより、いくつかのことを行っています。まず、apply_rangesメソッドをcallに配置することで、新しい機能を追加する場所を作っています。しかし、2番目に、そしておそらくより重要なこととして、現在の動作を維持するように、この新しい関数をすぐに実装しています。ある程度、このプレースホルダー関数を簡単に挿入できる機能は、強調表示の動作を一連の小さな変換として構成することの大きな利点の1つです。これは、パイプとフィルターパターンが計算を構成する非常に強力な方法である理由の1つです。

apply_rangesをこの単純な動作を維持する実装で定義することで、単に空白のままにするのではなく、テストを実行し続け、リファクタリングの帽子をかぶり続けることができます。

適用するハイライト範囲要素は任意に設定できるため、それぞれが他の要素の上に構成されるようにします。

class CodeHighlighter…

  def apply_ranges lines
    highlight_ranges.reduce(lines){|acc, each| apply_one_range(acc, each)}
  end
  def highlight_ranges
    @data.css('highlight-range')
  end
  def apply_one_range lines, element
    lines
  end

ここで、同じトリックを再び行っていることがわかります。apply_one_rangeを各ハイライト範囲要素に対して実行した結果を縮小することで、apply_rangesを実装します。既存の動作を維持するapply_one_rangeの初期実装を提供し、帽子をかぶり続けることができます。私がしていることは、追加しようとしている動作の変更の範囲を徐々に狭めていくことです。

この時点で、ハイライト範囲条件に対して何もしないテストを追加します。

class CodeHighlighterTester…

  def test_highlight_range_noop
    e = '<highlight-range start-line = "(date == null)" end-line = "}"/>'
    assert_equal hunks['input'], with_highlights(form_element(e))    
  end

これは奇妙な動きのように見えるかもしれません。基本的に、このテストは、ハイライト範囲要素を追加しても出力に変更を加えたくないことを示しているだけです。これは一時的なテストで、準備的なリファクタリングを行っている間だけです。このリファクタリングを行っている間、リファクタリングによって要素が存在する場合でも変更は発生しないという前提で動作しています。したがって、テストが非常に簡単なので、テストでその前提を確認したいのです。(これは私の一般的なルールに従っています:もしコードを実行して出力を見て、物事が正しいかどうかを確認したいという衝動を感じたら、代わりにテストを書くべきです。テストを使えば、コンピュータが出力が正しいかどうかを確認できるため、自分で確認する必要はありません。)

次の動きは、ハイライター自体に戻ります。これで、単一の範囲を強調表示するメソッドを分離しました。次に実行するのに良いことは、開始タグを追加する行を特定し、行を3つのリストに分割することです。一致した行の前、行単独、一致した行の後です。終了タグについては後で心配します。

class CodeHighlighter…

  def apply_one_range lines, element
    start_ix = lines.find_index {|line| line =~ Regexp.new(element['start-line'])}
    pre = 0 == start_ix ? [] : lines[0..(start_ix - 1)]
    start = [lines[start_ix]]
    rest = lines.size == (start_ix + 1) ? [] : lines[(start_ix + 1)..-1]
    return pre + start + rest
  end

このリファクタリングの本質は、テキストを分解して再構成し、新しい動作を滑り込ませるのに適切なポイントまで分解することです。

これにより、行のリストを正しく分割して再構成できることをテストできます。常に3つの部分があるとは限らないため、これは最初に考えるほど簡単ではありません。範囲が最初または最後から2番目の行から始まるかどうかを確認するために条件付きロジックを挿入する必要があったため、これらのケースを確認するためのテストを追加しました。

今のところ、開始のみを確認しており、実際に観察可能な動作を変更する準備がほぼできていますが、その前に、htmlのspan文字列をオブジェクトスコープにあるものに移動する必要があります。

class CodeHighlighter…

  def apply_markup line, element
    open = "<span class = 'highlight'>"
    close = "</span>"
    if element.key? 'span'
      r = Regexp.new(element['span'])
      m = r.match line
      raise "unable to match span %s" % element['span'] unless m
      m.pre_match + opening + m[0] + closing + m.post_match
    else
      opening + line.chomp + closing + "\n"
    end
  end

  def opening
    "<span class = 'highlight'>"
  end
  def closing
    "</span>"
  end

それらを定数にすることもできますが、このような状況ではメソッドを使用するのが私の習慣です。 [2]

これで、ついにハードハットをかぶる準備が整いました。そして、私が行う必要のある変更は、簡単にするには些細すぎます。

class CodeHighlighter…

  def apply_one_range lines, element
    start_ix = lines.find_index {|line| line =~ Regexp.new(element['start-line'])}
    raise "unable to match %s in code insert" % element['start-line'] unless start_ix
    pre = 0 == start_ix ? [] : lines[0..(start_ix - 1)]
    start = [opening + lines[start_ix]]
    rest = lines.size == (start_ix + 1) ? [] : lines[(start_ix + 1)..-1]
    return pre + start + rest
  end

ここで、数分前に追加した何もしないテストを削除し、スキップされたテストを変更して、開始のみを含めるようにします。

class CodeHighlighterTester…

  def test_highlight_range
    skip
    e = '<highlight-range start-line = "(date == null)" end-line = "}"/>'
    assert_equal hunks['range'], with_highlights(form_element(e))    
  end

codeHighlighterHunks.txt…

  %% range
  
    private void validateDate(Notification note) {
  <span class = 'highlight'>    if (date == null) {
        note.addError("date is missing");
        return;
      }</span>
  
      LocalDate parsedDate;
      try {
        parsedDate = LocalDate.parse(getDate());
      }
    } //end

このテストにより、終了タグを追加する前に、少し準備的なリファクタリングを行うことができます。

class CodeHighlighter…

  def apply_one_range lines, element
    start_ix = lines.find_index {|line| line =~ Regexp.new(element['start-line'])}
    raise "unable to match %s in code insert" % element['start-line'] unless start_ix
    finish_offset = lines[start_ix..-1].find_index do |line| 
      line =~ Regexp.new(element['end-line'])
    end
    raise "unable to match %s in code insert" % element['end-line'] unless finish_offset
    finish_ix = start_ix + finish_offset
    pre = 0 == start_ix ? [] : lines[0..(start_ix - 1)]
    start = [opening + lines[start_ix]]
    mid = (lines[(start_ix + 1)..(finish_ix -1)])
    finish = [lines[finish_ix]]
    rest = lines.size == (finish_ix + 1) ? [] : lines[(finish_ix + 1)..-1]
    return pre + start + mid + finish + rest
  end

この方法は私の好みからすると少々長すぎるが、どうすればうまく短くできるのか思いつかない。それでもすべてをグリーンに保ち、最後の簡単な変更のための準備を整えてくれる。

class CodeHighlighter…

  def apply_one_range lines, element
    start_ix = lines.find_index {|line| line =~ Regexp.new(element['start-line'])}
    raise "unable to match %s in code insert" % element['start-line'] unless start_ix
    finish_offset = lines[start_ix..-1].find_index do |line| 
      line =~ Regexp.new(element['end-line'])
    end
    raise "unable to match %s in code insert" % element['end-line'] unless finish_offset
    raise "start and end match same line" unless finish_offset > 0
    finish_ix = start_ix + finish_offset
    pre = 0 == start_ix ? [] : lines[0..(start_ix - 1)]
    start = [opening + lines[start_ix]]
    mid = (lines[(start_ix + 1)..(finish_ix -1)])
    finish = [lines[finish_ix].chomp + closing + "\n"]
    rest = lines.size == (finish_ix + 1) ? [] : lines[(finish_ix + 1)..-1]
    return pre + start + mid + finish + rest
  end

最終的な考察

この小さなエピソードが、準備的リファクタリングがどのようなものか、少しでも感じていただけたなら幸いです。

望む変更ごとに、変更を容易にし(注意:これは難しいかもしれない)、それから容易な変更を行う。

-- ケント・ベック

私は、与えられたものをそのまま返すだけのno-op関数を作成することで変更を容易にし、その関数を分解し、no-op性を維持しながら徐々に分解した。そして、新しい機能を追加するのが簡単になったら、それを滑り込ませるだけだった。

準備的リファクタリングのすべてのエピソードは異なります。数分で済むものもあれば、数日かかるものもあります。しかし、準備的リファクタリングの方法を見つけられると、より速く、ストレスの少ないプログラミング体験ができることがわかりました。なぜなら、ハードハットよりもトリルビーの方が速く、ストレスが少ないからです。


脚注

1: このアプローチは、このようなリファクタリングを説明しているときにはあまりうまくいかないという、面白い皮肉があります。私はコードのインポート機構を依然として使用しています。書いているときにコードをテキストから分離するのに便利だと感じているからです。しかし、(使用されなくなった)スニップファイルからインポートする必要があります。

2: ここで行っているように、どのようにしてこのコードでコードに打ち消し線を引いてハイライト表示できるのか疑問に思われるかもしれません。私はこの記事を書いているときに、コードハイライターにその機能を追加しました。(cssクラスを指定する属性を追加することで実現しました。)

重要な修正

2015年1月5日: 初版公開