SIerで仕事やっていてよく見るコードと直し方 - 依存性の高いコードのすこしばかりの改善

## まえがき

最近見たコードと自分で直した過程を妄想で思い出しながらメモとして残しています。
よくわからなったらすみません。  

タイトルの通り、依存性があるコードを使おうとして、動作確認に時間を費やしてしまったので、反省として記録しています。  
（Kotlinが流行っていると聞いたので、初心者コードなのですが書いてみました。）

## 改善対象のコードと問題点の定義

`SampleFileReader`と`SampleFileWriter`は、もともと用意されているコードです。**中身はテキトーなので、ここでは特に問題ないものとして考えてください**。

```kotlin
// 改善対象のクラス
class SampleManager(private val reader: SampleFileReader,
                    private val writer: SampleFileWriter) {
    
    fun doSomething1() {
        reader.read()
        writer.writeLine("てきとう do something1")
    }

    fun doSomething2() {
        reader.read()
        writer.writeLine("てきとう do something2")
    }
}

// 改善対象のクラス
class WorkFlow {
    private val readPath: String = "read.txt"
    private val writePath: String = "write.txt"
    
    // SampleManagerのインスタンスを利用できるようにしている。
    // テストしやすいように、依存関係を分離しておきたい。
    private val manager: SampleManager
        get() {
            val reader = SampleFileReader(readPath)
            val writer = SampleFileWriter(writePath)
            return SampleManager(reader, writer) //
        }

    // SampleManagerのインスタンスを使った処理
    fun output() {
        manager.doSomething1()
        manager.doSomething2()
    }
}

class SampleFileReader(private val filePath: String) {
    private val file: File?
        get() = Paths.get(filePath)?.toFile()

    fun read(): Iterator<String> {
        val br = BufferedReader(InputStreamReader(FileInputStream(file)))
        return br.readLines().iterator()
    }
}

class SampleFileWriter(private val filePath: String) {
    private val bw = BufferedWriter(OutputStreamWriter(FileOutputStream(file), "UTF-8"))

    private val file: File?
        get() = Paths.get(filePath)?.toFile()

    fun writeLine(str: String) = bw.write(str)

    fun close() = bw.close()
}
```

ここでの問題点は、以下1,2として定義しました。

**[問題点]**

1. `WorkFlow`クラスの中で`SampleManager`のインスタンスを生成しているのですが、`WorkFlow`クラスをテスト対象として考えた際、**`SampleManager`に依存しているため、テストがしづらいという問題があります**。
2. 実際に書いて動かしてみるとわかるのですが、`SampleManager`が依存している`SampleFileReader`で参照するファイルパスは実在しないとエラーになります。これも改善対象とします。

なので、WorkFlowのテストを考えたときに`SampleManager`のインスタンス生成処理を少し工夫して依存関係を分離します。

## 改善してみたコード


### 問題点1.の対応

`WorkFlow`クラスで`SampleManager`インスタンスを生成し、かつテストコードではテスト用に`SampleManager`インスタンスを設定できるように`WorkFlow`クラスを**継承可能**にする方針で考えました。  
具体的には、以下対応しました。

* `WorkFlow`クラスを継承可能にするために`open`で定義した
* インスタンス生成用のメソッド`createManager()`を`protected open`で追加定義した。
* `WorkFlow`クラスの代わりにテスト対象とする`TestWorkFlow`クラスをサブクラスとして定義し、`createManager()`メソッドをオーバーライドした。

本番用のコードをわざわざ`open`にするなよ。という気もしたのですが、私の業務上は`open`にして修正しても問題なさそうだったので`open`して継承を利用しました。

```kotlin
// これは次のステップで改善
class SampleManager(private val reader: SampleFileReader,
                    private val writer: SampleFileWriter) {
    
    fun doSomething1() {
        reader.read()
        writer.writeLine("てきとう do something1")
    }

    fun doSomething2() {
        reader.read()
        writer.writeLine("てきとう do something2")
    }
}

// Kotlinはデフォルトで継承不可なのでopenをつけています。
open class WorkFlow {
    private val readPath: String = "read.txt"
    private val writePath: String = "write.txt"
    
    private val manager: SampleManager
        get() {
            // ファクトリメソッドを利用する
            return createManager()
        }

    fun output() {
        manager.doSomething1()
        manager.doSomething2()
    }


    // ファクトリメソッドとして抽出
    // サブクラスでオーバーライドできるように修飾子を`protected open`としている
    protected open fun createManager(): SampleManager {
        val reader = SampleFileReader(readPath)
        val writer = SampleFileWriter(writePath)
        return SampleManager(reader, writer)
    }

}

// WorkFlowの代わりにテスト対象とするクラス
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        TODO("テスト用にSampleManagerのインスタンスを生成する")
    }
}
```

しかし、実際にテスト用にSampleManagerを返すようにTODOの部分を書こうとしたとき、
`SampleManager`が依存している`SampleFileReader`で参照するファイルパスは実在しないとエラーになります。  
`SampleManager`のインスタンスを生成できればいいのに、`SampleFileReader`, および`SampleFileWriter`も必要になってしまい、依存性が高いです。

```kotlin
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        val reader = SampleFileReader("読み込むために実在するパスを指定しないとだめ")
        val writer = SampleFileWriter("書き込み先のパス")
        return SampleManager(reader, writer)
    }
}

// テストコードでのインスタンス生成をイメージしていただければと..
fun main(args: Array<String>) {
    val flow = TestWorkFlow()
    flow.output()
}
```

なので、問題点2への対応として少し改善します。

### 問題点2.の対応

ここでは`SampleManager`の振る舞いをインタフェースとして抽出する方針で考えました。

```kotlin
// インタフェースとして振る舞いを抽出
interface SampleManager {
    fun doSomething1()
    fun doSomething2()
}

// SampleManagerをSampleManagerImplとしてクラス名を変更
class SampleManagerImpl(private val reader: SampleFileReader,
                        private val writer: SampleFileWriter) : SampleManager {

    override fun doSomething1() {
        val list = reader.read()
        println(list)
        writer.writeLine("てきとう do something1")
    }

    override fun doSomething2() {
        reader.read()
        writer.writeLine("てきとう do something2")
    }
}
```

テスト用のコードは以下になります。

```kotlin
// テスト用の擬似クラス。SampleManagerを実装。
// data classでなくても普通のクラスでいい。
data class FakeSampleManager(val prop1: String = "alice", 
                             val prop2: String = "bob") : SampleManager {
    override fun doSomething1() = println("$prop1 $prop1 $prop1")
    override fun doSomething2() = println("$prop2 $prop2 $prop2")
}

// WorkFlowの代わりにテスト対象とするクラス
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        // テスト用の擬似オブジェクトを返すように変更した。
        return FakeSampleManager()
    }
}
```

上のコードは`SampleFileReader`と`SampleFileWriter`を必要としていません。

これで、テスト時に「`SampleManager`のインスタンスを生成できればいいのに、`SampleFileReader`, および`SampleFileWriter`も必要になってしまい、依存性が高い」問題を解決できました。


### 全体のコード

参考までに全体のコードを載せておきます。

**本番用コード**

```kotlin
class SampleFileReader(private val filePath: String) {
    private val file: File?
        get() = Paths.get(filePath)?.toFile()

    fun read(): List<String> {
        val br = BufferedReader(InputStreamReader(FileInputStream(file)))
        return br.readLines()
    }
}


class SampleFileWriter(private val filePath: String) {
    private val bw = BufferedWriter(OutputStreamWriter(FileOutputStream(file), "UTF-8"))

    private val file: File?
        get() = Paths.get(filePath)?.toFile()

    fun writeLine(str: String) = bw.write(str)

    fun close() = bw.close()
}

interface SampleManager {
    fun doSomething1()
    fun doSomething2()
}

class SampleManagerImpl(private val reader: SampleFileReader,
                        private val writer: SampleFileWriter) : SampleManager {

    override fun doSomething1() {
        val list = reader.read()
        println(list)
        writer.writeLine("てきとう do something1")
    }

    override fun doSomething2() {
        reader.read()
        writer.writeLine("てきとう do something2")
    }
}


open class WorkFlow {
    private val readPath: String = "read.txt"
    private val writePath: String = "write.txt"

    private val manager: SampleManager
        get() {
            return createManager()
        }

    fun output() {
        manager.doSomething1()
        manager.doSomething2()
    }
    
    protected open fun createManager(): SampleManager {
        val reader = SampleFileReader(readPath)
        val writer = SampleFileWriter(writePath)
        return SampleManagerImpl(reader, writer)
    }

}
```

**テスト用**

```kotlin
data class FakeSampleManager(val prop1: String = "alice", 
                             val prop2: String = "bob") : SampleManager {
    override fun doSomething1() = println("$prop1 $prop1 $prop1")
    override fun doSomething2() = println("$prop2 $prop2 $prop2")
}

// WorkFlowの代わりにテスト対象とするクラス
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        return FakeSampleManager()
    }
}

// テスト時のインスタンス生成
fun testXX {
    val flow = TestWorkFlow()
    flow.output()
    ...
}
```

テストコードをそこまで書かない文化であったとしても、インスタンス生成において依存関係が分離されていて簡単にできるかどうかぐらいの検証を開発時にしておくことは、保守開発する人への思いやりとしてあってもいいな。と思う今日この頃です。

p.s. Kotlinで書いてみて、Scalaでおなか一杯なので暫くはいいかなと思いました。Scalaを経験していたこともあり、学習コストはとても低かったのです(文法2時間ぐらい)。機会があればいずれは業務で本格的に使ってみたいです。