The original article was published on August 8, 2024.
Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.
This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.
Long lifecycle entanglement
Suppose there is a class called FooBufferedLogger as shown below. The role of this class is to accumulate logs (LogElement) with bufferLogElement and send the accumulated logs all at once with close. Logs are categorized by tag, and if you want to change the tag, you need to create a new FooBufferedLogger instance. The FooApiClient specified in the constructor parameter is used for sending logs.
class FooBufferedLogger(
private val apiClient: FooApiClient,
private val tag: String
) : Closeable {
// `null` represents already closed.
private var buffer: MutableList<String>? = mutableListOf()
/**
* Buffers a given value as long as this logger is valid. Then, this returns true.
*
* The buffered values are sent to the server when [close] is called.
* If this buffer is already closed, this does nothing but returns false.
*/
fun bufferLogElement(logElement: String): Boolean {
val currentBuffer = buffer ?: return false
currentBuffer += logElement
return true
}
override fun close() {
val currentBuffer = buffer ?: return
buffer = null
apiClient.sendLogElements(tag, currentBuffer)
}
}The code that uses this is as follows. Since FooBufferedLogger inherits Closeable, it can be easily used with Kotlin's standard extension function use. (Similar mechanisms can be achieved in other languages with try-with-resource or ARC destructors.)
FooBufferedLogger(..., tag = "bar").use { logger ->
logger.bufferLogElement("start with $someValue")
...
logger.bufferLogElement("intermediate with $anotherValue")
...
logger.bufferLogElement("finished with $elapsedTimeInNanos")
}Suppose there is a requirement to "ensure that a common FooApiClient instance is used for multiple tags". In this case, it is natural to want to make FooBufferedLogger reusable for multiple tags and hide the management of FooApiClient and tag.
To meet this specification, the implementation of FooBufferedLogger was rewritten as follows.
class FooBufferedLogger(private val apiClient: FooApiClient) {
// `null` represents logging is not started.
private var tag: String? = null
private val buffer: MutableList<String> = mutableListOf()
/** ... (snip) ... */
fun startLogging(tag: String) {
finishLogging()
this.tag = tag
}
/** ... (snip) ... */
fun bufferLogElement(logElement: String): Boolean {
if (tag == null) {
return false
}
buffer += logElement
return true
}
/** ... (snip) ... */
fun finishLogging() {
val currentTag = tag ?: return
tag = null
apiClient.sendLogElements(currentTag, buffer)
buffer.clear()
}
}However, there is a problem with this implementation. What is it?
Too much spaghetti entanglement
The following code uses FooBufferedLogger. In executeBar and executeBaz, logging is performed with the tag of "Bar" and "Baz" respectively.
class BarBaz(private val logger: FooBufferedLogger) {
fun executeBar() = logWith("Bar") { logger ->
logger.bufferLogElement("bar 1")
...
executeBaz()
logger.bufferLogElement("bar 2")
}
private fun executeBaz() = logWith("Baz") { logger ->
logger.bufferLogElement("baz 1")
...
logger.bufferLogElement("baz 2")
}
private fun logWith(tag: String, action: (FooBufferedLogger) -> Unit) {
logger.startLogging(tag)
action(logger)
logger.finishLogging()
}
}Note that executeBar calls executeBaz. Since logger.startLogging("Baz") is called at the beginning of executeBaz, the "Bar" tag is overwritten. As a result, logger.bufferLogElement("bar 2") called when returning to executeBar is not executed correctly.
To solve this problem, it is conceivable to prepare multiple buffers and associate them with tag as a Map.
class FooBufferedLogger(private val apiClient: FooApiClient) {
private val categorizedLogElements: MutableMap<String, MutableList<String>> =
mutableMapOf()
...However, this solution still has the following problems:
- The
tagneeds to be provided in thebufferLogElementcall. - If
startLoggingis called multiple times with the sametag, the same problem occurs.
Separate by lifecycle length
The root cause of this problem is that the lifecycle of FooApiClient and tag are different, yet the scope of instance sharing was made the same. If you want to "ensure that a common FooApiClient is used for multiple tags", usually the lifecycle of tag is shorter than that of FooApiClient.
In such cases, separating the management of FooApiClient and tag into different classes is one solution. In the implementation below, an interface LogBuffer is created for managing tag. By making the implementation LogBufferImpl private, it ensures that a common FooApiClient is used.
class FooLogBufferFactory(private val apiClient: FooApiClient) {
/** ... */
fun createLogBuffer(tag: String): LogBuffer =
LogBufferImpl(apiClient, tag)
/** ... */
sealed interface LogBuffer : Closeable {
/** ... */
fun bufferLog(logElement: String): Boolean
}
private class LogBufferImpl(
private val apiClient: FooApiClient,
private val tag: String
) : LogBuffer {
// `null` represents already closed.
private var buffer: MutableList<String>? = mutableListOf()
override fun bufferLog(logElement: String): Boolean {
...
}
override fun close() {
...
}
}
}This implementation provides a function called createLogBuffer, but there are other solutions. It might be good to make it a higher-order function like fun logWith(tag: String, (LogBuffer) -> Unit).
In a nutshell
When managing items with different lifecycles, consider separating them into different classes.
Keywords: disposable instance, cyclic state, state lifecycle