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

というか一旦読み終わったが、とても読みやすい。感動しながら読んでいる。しかし思い返すと定期的に『Code Complete』とか読んで感動しているが何に感動したのかよく覚えていない。このままではこの本もそけっとさんが可愛すぎることしか思い出せなくなりそうなのでボチボチ知っている言語で写経してちょっとは糧としたい、していくことがこのスクラップの目的。
本の理解・コードの内容・引用の仕方など何かしら目に留まる部分あったら容赦なくマサカリを投げてもらえれば嬉しいです。

(love cat)

テスト駆動開発の章では例として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));
    }
}

(love cat)

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));
    }
}

(love cat)

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

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));
    }
}

(love cat)

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));
    }
}

(love cat)

最小限の労力で通す。

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

(love cat)

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)
            }
        }
    }
}

通った。

(love cat)

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));
    }
}

(love cat)

いよいよ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));
    }

(love cat)

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の倍数なら」の条件分岐を追加、テストにパス。

(love cat)

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

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}"),
        }
    }
}

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

(love cat)

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

(love cat)

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

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

(love cat)

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

src/core/replace_rule_interface.rs

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

(love cat)

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

src/core/number_converter.rs

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

(love cat)

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

(love cat)

traitにautomock。

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

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

(love cat)

テストというよりは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));
    }
}

(love cat)

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

    #[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));
    }

(love cat)

普通に自分のミスで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つ目の空ルールがコケるようになってしまった。

(love cat)

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

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)
        }
    }
}

(love cat)

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)

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

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

(love cat)

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

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))
    }

(love cat)

    #[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つのモックルールを使ったテストにもパス。完成しちゃったかな。

(love cat)

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

(love cat)

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));
    }
}

(love cat)

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

(love cat)

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

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));
    }
}

(love cat)

さて、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))
    }
}

(love cat)

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

(love cat)

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

#[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"が出力されてしまった。

(love cat)

今は全部のルールを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()
    }
}

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

(love cat)

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

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));
    }

(love cat)

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

(love cat)

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

(love cat)

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

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

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

(love cat)

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

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
    }
}

実装。

(love cat)

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

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()
    }
}

(love cat)

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

#[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));
    }
}

(love cat)

これにてテスト駆動開発の章は終わり。

今もFizzBuzzをこのように書いていくことができるかというと正直自信がないが、とはいえ読んだだけよりもちゃんと自分でも書いて追ってみたのはためになったように思う。
趣味で次に何か書くときはちゃんとテスト駆動開発を意識して書くぞという気にもなった。

このスクラップは2023/07/01にクローズされました