『ちょうぜつソフトウェア設計入門』を読むぞ：テスト駆動開発編

テスト駆動開発の章では例としてfizzbuzzを書いているのでそれをRustで真似していく。まずは失敗するところから。

src/core/number_converte.rs

pub struct NumberConverter {}

impl NumberConverter {
    pub fn convert(self, n: i32) -> String {
        format!("")
    }
}

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
    }
}

1, 2は数値を文字列にしてパス、3→fizzで当然コケる。

src/core/number_converter.rs

pub struct NumberConverter {}

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        format!("{}", n)
    }
}

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
    }
}

単純にパスさせる。テストが通るのはいつだって気持ちがいい。

src/core/number_converter.rs

pub struct NumberConverter {}

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n == 3 {
            format!("Fizz")
        } else {
            format!("{}", n)
        }
    }
}

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
    }
}

4は今の実装で問題なく通ったが、5でコケた。

src/core/number_converter.rs

pub struct NumberConverter {}

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n == 3 {
            format!("Fizz")
        } else {
            format!("{}", n)
        }
    }
}

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
        assert_eq!("4", fizzbuzz.convert(4));
        assert_eq!("Buzz", fizzbuzz.convert(5));
    }
}

最小限の労力で通す。

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n == 3 {
            format!("Fizz")
        } else {
            if n == 5 {
                format!("Buzz")
            } else {
                format!("{}", n)
            }
        }
    }
}

6はFizzになってほしい。コケた。

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
        assert_eq!("4", fizzbuzz.convert(4));
        assert_eq!("Buzz", fizzbuzz.convert(5));
        assert_eq!("Fizz", fizzbuzz.convert(6));
    }
}

Fizzを返すルールは現在n == 3としていた、これを3の倍数の場合へと変更する。

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n % 3 == 0 {
            format!("Fizz")
        } else {
            if n == 5 {
                format!("Buzz")
            } else {
                format!("{}", n)
            }
        }
    }
}

通った。

10までテストし、Buzzを返す場合についても実装を変更した。

pub struct NumberConverter {}

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n % 3 == 0 {
            format!("Fizz")
        } else {
            if n % 5 == 0 {
                format!("Buzz")
            } else {
                format!("{}", n)
            }
        }
    }
}

#[cfg(test)]
mod test {
    use super::NumberConverter;

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
        assert_eq!("4", fizzbuzz.convert(4));
        assert_eq!("Buzz", fizzbuzz.convert(5));
        assert_eq!("Fizz", fizzbuzz.convert(6));
        assert_eq!("7", fizzbuzz.convert(7));
        assert_eq!("8", fizzbuzz.convert(8));
        assert_eq!("Fizz", fizzbuzz.convert(9));
        assert_eq!("Buzz", fizzbuzz.convert(10));
    }
}

いよいよ15に挑戦。コケる。

    #[test]
    fn test_convert() {
        let fizzbuzz = NumberConverter {};
        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
        assert_eq!("4", fizzbuzz.convert(4));
        assert_eq!("Buzz", fizzbuzz.convert(5));
        assert_eq!("Fizz", fizzbuzz.convert(6));
        assert_eq!("7", fizzbuzz.convert(7));
        assert_eq!("8", fizzbuzz.convert(8));
        assert_eq!("Fizz", fizzbuzz.convert(9));
        assert_eq!("Buzz", fizzbuzz.convert(10));
        assert_eq!("FizzBuzz", fizzbuzz.convert(15));
    }

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        if n % 3 == 0 {
            if n % 5 == 0 {
                format!("FizzBuzz")
            } else {
                format!("Fizz")
            }
        } else {
            if n % 5 == 0 {
                format!("Buzz")
            } else {
                format!("{}", n)
            }
        }
    }
}

先程は"Fizz"が返されたので、どこの分岐に入ったのかは明らかであった。
そこで"Fizz"を返される前に「5の倍数なら」の条件分岐を追加、テストにパス。

愚直に分岐をポコポコ足してきたのでそろそろリファクタリングをしたい。
テストコードはここまででいい感じになっているので、リファクタリング後もテストにパスをするならちゃんと動作していると言えるはず。

impl NumberConverter {
    pub fn convert(&self, n: i32) -> String {
        match (n % 3, n % 5) {
            (0, 0) => format!("FizzBuzz"),
            (0, _) => format!("Fizz"),
            (_, 0) => format!("Buzz"),
            _ => format!("{n}"),
        }
    }
}

テストにも通った。めでたしめでたし。

と言いたくなってしまうが、これはBDD（振る舞い駆動開発）の域をでないとのこと。BDDの目的は実装、しかしTDDは設計を目的とする。
先程のコードはFizzBuzzのコードであり、一般的にFizzBuzzといえば3や5の倍数が変換されるものであるが、ちょっとそこの固定観念は置いといて。
「4とか9こそ縁起が悪いから置き換えたい」「FizzとかBuzzって何？日本人ならhogeにfugaでしょ」とお偉いさんに対するレビューでぶっこまれたりする可能性がある。
そういった変更に強いコードをTDDで設計していきたい。

FizzBuzzの最小構成単位は、「何らかの単純な法則で、整数を文字列に置換するルールがある」という感じの抽象で、全体としては、その抽象を好きに組み立てできるものになっていると便利そうだな、と感じます。

この抽象化の能力がほしい。TDDを続けているとできるものなのだろうか。

とにかく、その抽象を記述する。

src/core/replace_rule_interface.rs

pub trait ReplaceRuleInterface {
    fn replace(&self, n: i32) -> String;
}

また、NumberConverter構造体をルールの集合体と仮定。

src/core/number_converter.rs

pub struct NumberConverter<T: ReplaceRuleInterface> {
    rules: Vec<T>,
}

抽象化の能力が欲しいだのないものねだりする前に、まずテストを行う。それがTDDだと言われてる気がする。なるほど。そして今は抽象だけがあって実装がない。モックが必要なのでこちらの使い方をまずは知る。
https://docs.rs/mockall/latest/mockall/

traitにautomock。

#[cfg(test)]
use mockall::automock;

#[cfg_attr(test, automock)]
pub trait ReplaceRuleInterface {
    fn replace(&self, n: i32) -> String;
}

テストというよりはNumberConverterが動かせることを確認、みたいな気分ではあるが。
境界条件「ルールなし」で空文字が出ることを確認。

impl<T: ReplaceRuleInterface> NumberConverter<T> {
    pub fn new() -> Self {
        Self { rules: vec![] }
    }

    pub fn convert(&self, n: i32) -> String {
        format!("")
    }
}

#[cfg(test)]
mod test {
    use super::replace_rule_interface::*;
    use super::NumberConverter;

    #[test]
    fn test_convert_empty_rule() {
        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> = NumberConverter::new();

        assert_eq!("", fizzbuzz.convert(1));
    }
}

ちゃんとモックを使ってテストを追加してみる。今の実装は空文字を返すだけなのでコケる。

    #[test]
    fn test_convert_with_single_rule() {
        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> = NumberConverter::new();
        let mut mock = MockReplaceRuleInterface::new();
        mock.expect_replace().with(eq(1)).return_const("Replaced");

        assert_eq!("Replaced", fizzbuzz.convert(1));
    }

普通に自分のミスでNumberConverterがルール受け取るようになってなかった。というわけでnewを書き直して、rulesの1つ目を使って変換を試みるように。

impl<T: ReplaceRuleInterface> NumberConverter<T> {
    pub fn new(rules: Vec<T>) -> Self {
        Self { rules }
    }

    pub fn convert(&self, n: i32) -> String {
        self.rules[0].replace(n)
    }
}

#[cfg(test)]
mod test {
    use super::replace_rule_interface::*;
    use super::NumberConverter;
    use mockall::predicate::*;

    #[test]
    fn test_convert_with_empty_rule() {
        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> = NumberConverter::new(vec![]);

        assert_eq!("", fizzbuzz.convert(1));
    }

    #[test]
    fn test_convert_with_single_rule() {
        let mut mock = MockReplaceRuleInterface::new();
        mock.expect_replace().with(eq(1)).return_const("Replaced");

        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> = NumberConverter::new(vec![mock]);

        assert_eq!("Replaced", fizzbuzz.convert(1));
    }
}

1つ目の空ルールがコケるようになってしまった。

とにかく今あるテストをパスさせる。空だったときの分岐を追加。両方のテストにパスしたことを確認。

impl<T: ReplaceRuleInterface> NumberConverter<T> {
    pub fn new(rules: Vec<T>) -> Self {
        Self { rules }
    }

    pub fn convert(&self, n: i32) -> String {
        if self.rules.is_empty() {
            format!("")
        } else {
            self.rules[0].replace(n)
        }
    }
}

2個以上のルールだとどうなるかな？当然コケる。mock作成は切り出した。

    #[test]
    fn test_convert_with_fizz_buzz_rules() {
        let mock_fizz = create_mock_rule(1, "Fizz");
        let mock_buzz = create_mock_rule(1, "Buzz");

        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> =
            NumberConverter::new(vec![mock_fizz, mock_buzz]);

        assert_eq!("FizzBuzz", fizzbuzz.convert(1));
    }

    fn create_mock_rule(expected_number: i32, replacement: &str) -> MockReplaceRuleInterface {
        let mut mock = MockReplaceRuleInterface::new();
        mock.expect_replace()
            .with(eq(expected_number))
            .return_const(replacement.to_string());
        mock
    }

テストを記述している間に、仕様をより一般化できることに気づきます。

与えられた値が複数のルールに当てはまるとき、どうすればいいか。
両方の文字列を連結したものが返されるのが正解のようだ。

(love cat)2023/06/30に更新

とりあえずテストにパス。効率はどうなんだろう。気にしない。

impl<T: ReplaceRuleInterface> NumberConverter<T> {
    pub fn new(rules: Vec<T>) -> Self {
        Self { rules }
    }

    pub fn convert(&self, n: i32) -> String {
        self.rules
            .iter()
            .fold(String::from(""), |carry, rule| carry + &rule.replace(n))
    }

    #[test]
    fn test_convert_with_unmatched_fizzbuzz_rules_and_constant_rule() {
        let mock_1 = create_mock_rule(1, "");
        let mock_2 = create_mock_rule(1, "");
        let mock_3 = create_mock_rule(1, "1");

        let fizzbuzz: NumberConverter<MockReplaceRuleInterface> =
            NumberConverter::new(vec![mock_1, mock_2, mock_3]);

        assert_eq!("1", fizzbuzz.convert(1));
    }

3つのモックルールを使ったテストにもパス。完成しちゃったかな。

じゃあ本物の実装を作っていくか〜！

src/spec/cyclic_number_rule.rs

use crate::core::replace_rule_interface::ReplaceRuleInterface;

struct CyclicNumberRule {
    base: i32,
    replacement: String,
}

impl CyclicNumberRule {
    fn new(base: i32, replacement: &str) -> Self {
        Self {
            base,
            replacement: replacement.to_string(),
        }
    }
}

impl ReplaceRuleInterface for CyclicNumberRule {
    fn replace(&self, n: i32) -> String {
        if n % self.base == 0 {
            self.replacement.clone()
        } else {
            "".to_string()
        }
    }
}

#[cfg(test)]
mod test {
    use super::*;
    #[test]
    fn test_replace() {
        let rule = CyclicNumberRule::new(3, "Fizz");
        assert_eq!("", rule.replace(1));
        assert_eq!("Fizz", rule.replace(3));
        assert_eq!("Fizz", rule.replace(6));
    }
}

今日はここまで。明日はP155から、数字を文字列にするクラスを作ったりする。

与えられた数値をそのまま文字列にして返すルールも作成。

src/spec/pass_through_rule.rs

use crate::core::replace_rule_interface::ReplaceRuleInterface;

struct PassThroughRule {}

impl PassThroughRule {
    pub fn new() -> Self {
        PassThroughRule {}
    }
}

impl ReplaceRuleInterface for PassThroughRule {
    fn replace(&self, n: i32) -> String {
        n.to_string()
    }
}

#[cfg(test)]
mod test {
    use super::*;

    #[test]
    fn test_replace() {
        let rule = PassThroughRule::new();
        assert_eq!("1", rule.replace(1));
    }
}

さて、FizzBuzzを組み立てていく、まずは当然テストから…と思ったが構造体の定義が間違っていた。
複数ルールを持つためにトレイトオブジェクトで書き直す。反省。

pub struct NumberConverter {
    rules: Vec<Box<dyn ReplaceRuleInterface>>,
}

impl NumberConverter {
    pub fn new(rules: Vec<Box<dyn ReplaceRuleInterface>>) -> Self {
        Self { rules }
    }

    pub fn convert(&self, n: i32) -> String {
        self.rules
            .iter()
            .fold(String::from(""), |carry, rule| carry + &rule.replace(n))
    }
}

ちょちょっと書き直して、エラーが出てるテストを修正して、ぽんとテストを流せば全部これまで通りに動いているなと安心できるのはいいなと実感。

ルールを組み立てられるようになったので改めてテスト。

#[cfg(test)]
mod test {
    use crate::{
        core::{number_converter::NumberConverter, replace_rule_interface::ReplaceRuleInterface},
        spec::{cyclic_number_rule::CyclicNumberRule, pass_through_rule::PassThroughRule},
    };

    #[test]
    fn test_fizzbuzz() {
        let rules: Vec<Box<dyn ReplaceRuleInterface>> = vec![
            Box::new(CyclicNumberRule::new(3, "Fizz")),
            Box::new(CyclicNumberRule::new(5, "Buzz")),
            Box::new(PassThroughRule::new()),
        ];
        let fizzbuzz = NumberConverter::new(rules);

        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
    }
}

コケる。3のときに"Fizz3"が出力されてしまった。

今は全部のルールを1つずつ通して、その結果を前の結果に足すようにしている。
PassThroughRuleでは前のルールに合致したものを無視するようにしないとだめなんだな。

impl ReplaceRuleInterface for PassThroughRule {
    fn replace(&self, n: i32) -> String {
        if n % 3 == 0 || n % 5 == 0 {
            return "".to_string();
        }
        n.to_string()
    }
}

とするのはもちろんダメ。ここは変更に対してクローズドでいたい部分なのに、変更が発生するかもしれないと思ってるマジックナンバーを埋め込むのはいけない。
急ぎのテストが予想外に失敗したときに同じことをしないと言い切れないのが怖い。どうしてダメなのか、肝に銘じる。

実装の深いところにハードコードされてるのがよくない。ならどの数を無視するのか設定できるようにしよう。

impl PassThroughRule {
    pub fn new(exceptional_numbers: Vec<i32>) -> Self {
        PassThroughRule {
            exceptional_numbers,
        }
    }
}

impl ReplaceRuleInterface for PassThroughRule {
    fn replace(&self, n: i32) -> String {
        if self.exceptional_numbers.iter().any(|&x| n % x == 0) {
            return "".to_string();
        }
        n.to_string()
    }
}

テスト内だけど、使い方はこうなる。

    #[test]
    fn test_fizzbuzz() {
        let rules: Vec<Box<dyn ReplaceRuleInterface>> = vec![
            Box::new(CyclicNumberRule::new(3, "Fizz")),
            Box::new(CyclicNumberRule::new(5, "Buzz")),
            Box::new(PassThroughRule::new(vec![3, 5])),
        ];
        let fizzbuzz = NumberConverter::new(rules);

        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
    }

しかし、これは悪くないけど避けたほうがいいコードの例なのであった。
変換のルールで指定したパラメータをさらに無視するパラメータとして指定しており、DRYでないと。
妥協するな、技術的負債だぞ、と注意されている。

そもそも、与えられた値が複数のルールに当てはまるとき、両方の文字列を連結したものが返されるのが正解、という仮設が誤っていたっぽい。
これは実際にテストするなかで初めて気がつけたもので、動かしながら設計を洗練していこうってコト。

(love cat)2023/07/01に更新

今のReplaceRuleIInterfaceが与えられた整数をなんらかの文字列に変換するだけ。
そのために「他のルールに合致しなかった場合」をうまいこと扱えない。

まだ存在しない架空の要求仕様、どんなものがあるか考えてみると、「入力値を使って現在の値を書き換えるもの」とするのがいいかもしれない。
判定を増やすので、判定と処理を別々のメソッドにするといいかも。

pub trait ReplaceRuleInterface {
    fn apply(&self, carry: &str, n: i32) -> String;
    fn match_check(&self, carry: &str, n: i32) -> bool;
}

(love cat)2023/07/01に更新

当然実装側がぶっ壊れるので直していく。

mod test {
    use super::*;
    #[test]
    fn test_apply() {
        let rule = CyclicNumberRule::new(0, "Buzz");

        assert_eq!("Buzz", rule.apply("", 0));
        assert_eq!("FizzBuzz", rule.apply("Fizz", 0));
    }

    #[test]
    fn test_match_check() {
        let rule = CyclicNumberRule::new(3, "");

        assert!(!rule.match_check("", 1));
        assert!(rule.match_check("", 3));
        assert!(rule.match_check("", 6));
    }
}

まずテストを作って…。

impl ReplaceRuleInterface for CyclicNumberRule {
    fn apply(&self, carry: String, _n: i32) -> String {
        carry + &self.replacement
    }

    fn match_check(&self, _carry: &str, n: i32) -> bool {
        n % self.base == 0
    }
}

実装。

それまでのルールで変換されていない場合のみに数値を文字列にするルール。

impl ReplaceRuleInterface for PassThroughRule {
    fn apply(&self, _carry: String, n: i32) -> String {
        n.to_string()
    }

    fn match_check(&self, carry: &str, _n: i32) -> bool {
        carry.is_empty()
    }
}

シンプルにルールを追加できるようになった。テストにももう通る。

#[cfg(test)]
mod test {
    use crate::{
        core::{number_converter::NumberConverter, replace_rule_interface::ReplaceRuleInterface},
        spec::{cyclic_number_rule::CyclicNumberRule, pass_through_rule::PassThroughRule},
    };

    #[test]
    fn test_fizzbuzz() {
        let rules: Vec<Box<dyn ReplaceRuleInterface>> = vec![
            Box::new(CyclicNumberRule::new(3, "Fizz")),
            Box::new(CyclicNumberRule::new(5, "Buzz")),
            Box::new(PassThroughRule::new()),
        ];
        let fizzbuzz = NumberConverter::new(rules);

        assert_eq!("1", fizzbuzz.convert(1));
        assert_eq!("2", fizzbuzz.convert(2));
        assert_eq!("Fizz", fizzbuzz.convert(3));
        assert_eq!("4", fizzbuzz.convert(4));
        assert_eq!("Buzz", fizzbuzz.convert(5));
        assert_eq!("Fizz", fizzbuzz.convert(6));
        assert_eq!("Fizz", fizzbuzz.convert(9));
        assert_eq!("Buzz", fizzbuzz.convert(10));
        assert_eq!("FizzBuzz", fizzbuzz.convert(15));
        assert_eq!("FizzBuzz", fizzbuzz.convert(30));
    }
}