[PYTHON] Codes that you often see when working with SIer and how to fix them-Slight improvements to highly dependent code

Preface

I leave it as a memo while remembering the code I saw recently and the process of fixing it myself. I'm sorry if I don't understand.

As the title suggests, I spent a lot of time checking the operation while trying to use dependent code, so I'm recording it as a reflection. (I heard that Kotlin is popular, so I wrote it though it is a beginner code.)

Definition of code to be improved and problem

SampleFileReader and SampleFileWriter are the code that was originally provided. ** The contents are texto, so please consider it as no problem here **.

//Class to be improved
class SampleManager(private val reader: SampleFileReader,
                    private val writer: SampleFileWriter) {
    
    fun doSomething1() {
        reader.read()
        writer.writeLine("Kitto do something1")
    }

    fun doSomething2() {
        reader.read()
        writer.writeLine("Kitto do something2")
    }
}

//Class to be improved
class WorkFlow {
    private val readPath: String = "read.txt"
    private val writePath: String = "write.txt"
    
    //An instance of SampleManager is available.
    //I want to keep the dependencies separate for easy testing.
    private val manager: SampleManager
        get() {
            val reader = SampleFileReader(readPath)
            val writer = SampleFileWriter(writePath)
            return SampleManager(reader, writer) //
        }

    //Processing using an instance of 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()
}

The problems here are defined as 1 and 2 below.

[problem]

  1. I have created an instance of SampleManager in the WorkFlow class, but when I think of the WorkFlow class as a test target, it depends on ** SampleManager, so it is tested. There is a problem that it is difficult **.
  2. As you can see by actually writing and moving it, an error will occur if the file path referenced by SampleFileReader on which SampleManager depends does not actually exist. This is also subject to improvement.

Therefore, when considering the WorkFlow test, the instance creation process of SampleManager is slightly devised to separate the dependencies.

Code I tried to improve

Response to problem 1.

I thought about making the WorkFlow class ** inheritable ** so that I could create a SampleManager instance with the WorkFlow class and set the SampleManager instance for testing in the test code. Specifically, the following measures have been taken.

Don't bother to make the production code ʻopen. I thought that, but in my business, it seemed that there would be no problem even if I changed it to ʻopen, so I used ʻopen` and inherited.

//This will be improved in the next step
class SampleManager(private val reader: SampleFileReader,
                    private val writer: SampleFileWriter) {
    
    fun doSomething1() {
        reader.read()
        writer.writeLine("Kitto do something1")
    }

    fun doSomething2() {
        reader.read()
        writer.writeLine("Kitto do something2")
    }
}

//Kotlin is non-inheritable by default, so open is added.
open class WorkFlow {
    private val readPath: String = "read.txt"
    private val writePath: String = "write.txt"
    
    private val manager: SampleManager
        get() {
            //Use factory methods
            return createManager()
        }

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


    //Extracted as a factory method
    //Qualifiers that can be overridden in subclasses`protected open`To
    protected open fun createManager(): SampleManager {
        val reader = SampleFileReader(readPath)
        val writer = SampleFileWriter(writePath)
        return SampleManager(reader, writer)
    }

}

//Class to be tested instead of WorkFlow
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        TODO("Create an instance of SampleManager for testing")
    }
}

However, when I tried to write the TODO part to actually return the SampleManager for testing, An error will occur if the file path referenced by the SampleFileReader on which the SampleManager depends does not exist. I wish I could create an instance of SampleManager, but I also needSampleFileReader, and SampleFileWriter, which is highly dependent.

class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        val reader = SampleFileReader("You have to specify an existing path to read")
        val writer = SampleFileWriter("Write destination path")
        return SampleManager(reader, writer)
    }
}

//I hope you can imagine the instantiation in the test code...
fun main(args: Array<String>) {
    val flow = TestWorkFlow()
    flow.output()
}

So, I will improve it a little as a response to problem 2.

Response to problem 2.

Here, I thought about the policy of extracting the behavior of SampleManager as an interface.

//Extract behavior as an interface
interface SampleManager {
    fun doSomething1()
    fun doSomething2()
}

//Renamed the class with SampleManager as SampleManagerImpl
class SampleManagerImpl(private val reader: SampleFileReader,
                        private val writer: SampleFileWriter) : SampleManager {

    override fun doSomething1() {
        val list = reader.read()
        println(list)
        writer.writeLine("Kitto do something1")
    }

    override fun doSomething2() {
        reader.read()
        writer.writeLine("Kitto do something2")
    }
}

The code for the test is below.

//Pseudo class for testing. Implemented SampleManager.
//It doesn't have to be a data class, it can be an ordinary 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")
}

//Class to be tested instead of WorkFlow
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        //Changed to return a pseudo object for testing.
        return FakeSampleManager()
    }
}

The above code does not require SampleFileReader and SampleFileWriter.

This solved the problem of "I wish I could create an instance of SampleManager, but I also needed SampleFileReader, and SampleFileWriter, which is highly dependent. "

Whole code

For reference, I'll put the whole code here.

** Production code **

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("Kitto do something1")
    }

    override fun doSomething2() {
        reader.read()
        writer.writeLine("Kitto 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)
    }

}

for test

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

//Class to be tested instead of WorkFlow
class TestWorkFlow : WorkFlow() {
    override fun createManager(): SampleManager {
        return FakeSampleManager()
    }
}

//Instantiation during testing
fun testXX {
    val flow = TestWorkFlow()
    flow.output()
    ...
}

Even in a culture where test code is not written so much, it is a consideration for maintenance developers to verify at the time of development whether the dependencies are separated and easy to do in instance generation. It's okay. I think it's around this time today.

I wrote it in p.s. Kotlin and thought it would be good for a while because I'm full with Scala. Since I had experienced Scala, the learning cost was very low (about 2 hours of grammar). If I have a chance, I would like to use it in earnest in my business.

Recommended Posts

Codes that you often see when working with SIer and how to fix them-Slight improvements to highly dependent code
Codes I often see while working at SIer and how to fix them-Demeter's law violation
How to deal with the error "Failed to load module" canberra-gtk-module "that appears when you run OpenCV
How to deal with errors when installing whitenoise and deploying to Heroku
How to deal with errors when installing Python and pip with choco
How to build Python and Jupyter execution environment with VS Code
Convert facial images with PULSE to high image quality so that you can see pores and texture