はじめに
リンクアンドモチベーションでソフトウェアエンジニアをしているkoboriです。
つい最近、研修でコード改善の講義を受けたので、その復習も兼ね、なるべく読みにくいレガシーコードを自分で書き、自分で改善をして、その過程を記事にすることにしました。
レガシーコードを用意する
まず初めになるべく読みにくいレガシーコードを作成していきます。ここでいう レガシーコードとは、 テストがないコードを指しています。
ただ、自分で書いたテストがないコードを自分で改善するのでは、少し改善の難易度が低いと思ったので、下記の4点に留意して可読性を大幅に下げました。
- テストは絶対に書かない
- 変数はなるべく短く、省略する
- マジックナンバーを積極的に多用する
- クラスとメソッドはなるべく大きくする
- このコードを書いてから改善するまで3日以上の間隔を開ける
その結果書かれたコードがこちらです。
class Minesweeper
def self.start
x1 = [{ d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }]
x2 = [{ d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }]
x3 = [{ d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }, { d: "■", n: 0, m: false, f: false }]
b = [x1, x2, x3]
mine1 = [rand(0..2), rand(0..2)]
mine2 = [rand(0..2), rand(0..2)]
loop do
break if mine1 != mine2
mine2 = [rand(0..2), rand(0..2)]
end
b[mine1[0]][mine1[1]][:m] = true
b[mine2[0]][mine2[1]][:m] = true
(0..2).each do |i|
(0..2).each do |j|
if b[i][j][:m] == false
top = i - 1
down = i + 1
left = j - 1
right = j + 1
if top >= 0
b[i][j][:n] += 1 if b[top][j][:m] == true
end
if down <= 2
b[i][j][:n] += 1 if b[down][j][:m] == true
end
if left >= 0
b[i][j][:n] += 1 if b[i][left][:m] == true
end
if right <= 2
b[i][j][:n] += 1 if b[i][right][:m] == true
end
top_left = [top, left]
top_right = [top, right]
down_left = [down, left]
down_right = [down, right]
if top_left[0] >= 0 && top_left[1] >= 0
b[i][j][:n] += 1 if b[top_left[0]][top_left[1]][:m] == true
end
if top_right[0] >= 0 && top_right[1] <= 2
b[i][j][:n] += 1 if b[top_right[0]][top_right[1]][:m] == true
end
if down_left[0] <= 2 && down_left[1] >= 0
b[i][j][:n] += 1 if b[down_left[0]][down_left[1]][:m] == true
end
if down_right[0] <= 2 && down_right[1] <= 2
b[i][j][:n] += 1 if b[down_right[0]][down_right[1]][:m] == true
end
end
end
end
puts "マインスイーパを開始します。"
puts "爆弾は全部で#{2}個あります。"
puts "回答を入力してください。(例: 1,1)"
puts b.map { |x| x.map { |s| s[:d] }.join(" ") }
loop do
line = gets.split(',')
x = line[0].to_i
y = line[1].to_i
if b[x-1][y-1][:m] == true
puts "爆弾です。"
break
end
b[x-1][y-1][:d] = b[x-1][y-1][:n]
cl = b.all? do |x|
x.all? do |h|
h[:d] != "■" || h[:m] == true
end
end
if cl == true
puts "---"
puts "クリアです。"
puts "---"
puts b.map { |x| x.map { |s| s[:m] == true ? "✖" : s[:d] }.join(" ") }
break
end
puts "---"
puts "次の回答を入力してください。"
puts "---"
puts b.map { |x| x.map { |y| y[:d] }.join(" ") }
end
puts "マインスイーパを終了します。"
end
end
どんなアプリケーションか
作成したアプリケーションの説明を簡単に書きます。
課題となるアプリケーションの内容は、CUIで遊べるマインスイーパーにしました。
主な仕様は下記の通りです。
- 地雷原のマスは3×3で固定
- 地雷の数は2個で固定
-
1,1のような形式のカンマ区切りで入力し、オープンする地雷を決定する - 地雷のマスをオープンすると、ゲームオーバーになる
- 地雷以外のマスを全てオープンすると、クリアになる
改善のゴールを決める
改善を始めるにあたり、今回のゴールを下記で定めました。
- Minesweeperクラスを読める状態にする
- 単体テストを書くことができる状態にする
改善スタート
課題となるレガシーコードを書いてから4日経ったので、改善を始めていきます。
今回の改善作業の全体感は下記の通りです。
- 最低限の挙動を担保するテストを追加する
- Misweeperクラスの処理を別クラスに切り出す
これらの作業の過程で、変数名の修正等もやっていきます。
①最低限の挙動を担保するテストを追加する
まず初めにRspecを導入し、現在の挙動を担保する最低限のテストを追加します。
爆弾の位置と入力値をモックで固定化し、ゲームをクリアできた場合とクリアできなかった場合の2パターンのテストを追加しました。検査の対象は標準出力にしています。
それぞれのケースのテストコードは下記の通りです。
地雷を踏みゲームオーバーになった場合
context 'クリアできなかった場合' do
it '爆弾があったことを示す文字列が出力され、ゲームが中断される' do
allow(Minesweeper).to receive(:rand).and_return(0, 0, 1, 1)
allow(ARGF).to receive(:gets).and_return(StringIO.new('1,1').gets)
expected_stdout = <<~EOS
マインスイーパを開始します。
爆弾は全部で2個あります。
回答を入力してください。(例: 1,1)
■ ■ ■
■ ■ ■
■ ■ ■
爆弾です。
マインスイーパを終了します。
EOS
expect { Minesweeper.start }.to output(expected_stdout).to_stdout
end
end
地雷を踏まずゲームクリアできた場合
context 'クリアできた場合' do
it 'マスがすべて開き、正常にゲームが終了する' do
allow(Minesweeper).to receive(:rand).and_return(0, 0, 1, 1)
allow(ARGF).to receive(:gets).and_return(
StringIO.new('1,2').gets,
StringIO.new('1,3').gets,
StringIO.new('2,1').gets,
StringIO.new('2,3').gets,
StringIO.new('3,1').gets,
StringIO.new('3,2').gets,
StringIO.new('3,3').gets,
)
expected_stdout = <<~EOS
マインスイーパを開始します。
爆弾は全部で2個あります。
回答を入力してください。(例: 1,1)
■ ■ ■
■ ■ ■
■ ■ ■
---
次の回答を入力してください。
---
■ 2 ■
■ ■ ■
■ ■ ■
---
次の回答を入力してください。
---
■ 2 1
■ ■ ■
■ ■ ■
---
次の回答を入力してください。
---
■ 2 1
2 ■ ■
■ ■ ■
---
次の回答を入力してください。
---
■ 2 1
2 ■ 1
■ ■ ■
---
次の回答を入力してください。
---
■ 2 1
2 ■ 1
1 ■ ■
---
次の回答を入力してください。
---
■ 2 1
2 ■ 1
1 1 ■
---
クリアです。
---
✖ 2 1
2 ✖ 1
1 1 1
マインスイーパを終了します。
EOS
expect { Minesweeper.start }.to output(expected_stdout).to_stdout
end
end
追加したテストケースを実行し、無事通ることを確認できました。
②Misweeperクラスの処理を別クラスに切り出す
テストで足場を固め、レガシーコードと戦える体制が整ったので、Minesweeperクラスの改善に移ります。
テストの結果を頼りに、読みにくい変数名やキーの名前を修正していきつつ、Minesweeperクラスの処理を眺めていると、下記のように処理を分割できそうなポイントが見えてきました。
- 盤面の初期状態を作る
- 盤面を表示する
- 盤面をオープンする
- 地雷の判定をする
- ゲームクリアの判定をする
その結果、盤面に対して操作をしている箇所が多いことが分かったため、Boardクラスを作成し、各メソッドのインターフェースを考えました。
class Board
attr_reader :squires
def initialize(mine_count:, row_count:, column_count:)
# 爆弾の数、盤面の縦と横の数を受け取り、盤面の初期状態を作る
end
def current_board
# 現在のボードの状態を返す
end
def mine?(row_number:, column_number:)
# 盤面の座標を受け取り、爆弾かどうかの判定をする
end
def open(row_number:, column_number:)
# 盤面の座標を受け取り、オープンする
end
def completed?
# クリア状態かどうかの判定をする
end
def completed_board
# クリア後のボードの状態を返す
end
end
あとは、Minesweeperクラスの処理を、1つずつテストを確認しながらBoardクラスへ切り出していき、ゲームにおける主要な処理の単体テストを書くことができる状態になりました。
余談ですが、利用者として ゲームの難易度を上げたかったため、爆弾と盤面のマス数を可変にする対応を、この過程で併せて実施しました。
詳細は下記に置いてあります。
改善後のMinesweeperクラス
命名を修正し、処理のまとまりに名前をつけて切り出す。この2つの作業でMinesweeperクラスをすっきりさせ、改善前と比べて読みやすいコードに修正することができました。
決してベストな状態であるとは言えないですが、今後の改善のスタートラインに立つことができたのではないかと思っています。
require './board'
class Minesweeper
MINE_COUNT = 2
ROW_COUNT = 3
COLUMN_COUNT = 3
def self.start
board = Board.new(
mine_count: MINE_COUNT,
row_count: ROW_COUNT,
column_count: COLUMN_COUNT,
)
puts "マインスイーパを開始します。"
puts "爆弾は全部で#{MINE_COUNT}個あります。"
puts "回答を入力してください。(例: 1,1)"
puts board.current_board
loop do
line = gets.split(',')
row_input = line[0].to_i - 1
column_input = line[1].to_i - 1
if board.mine?(row_number: row_input, column_number: column_input)
puts "爆弾です。"
break
end
board.open(row_number: row_input, column_number: column_input)
if board.completed?
puts "---"
puts "クリアです。"
puts "---"
puts board.completed_board
break
end
puts "---"
puts "次の回答を入力してください。"
puts "---"
puts board.current_board
end
puts "マインスイーパを終了します。"
end
end
さいごに
コードを改善する際には、修正作業の前にまず信頼できるテストが何より先に必要であることを実感することができました。
最後に改善の前後のソースの置き場所を貼っておきます。
改善前
改善後