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.
Knock on the constructor for good luck
The following FooVideoPlayer class is used to play videos. You can call FooVideoPlayer.play to play a video, but you need to call prepare first to set the preparedValue. If you call play without calling prepare, an exception will be thrown due to an error.
/**
* A video player for a file specified by [videoUri].
*
* Call [play] to play the video, but [prepare] must be called before calling `play`.
* Otherwise `play` will throw [IllegalStateException].
*/
class FooVideoPlayer(
private val videoUri: Uri,
... // other options
) {
private var preparedValue: PreparedValue? = null
fun prepare() {
if (preparedValue != null) {
error("Already prepared")
}
preparedValue = ... // execute `prepare` logic
}
fun play() {
val currentValue = preparedValue
if (currentValue == null) {
error("Not prepared yet")
}
// ... play `videoUri`.
}
}Can this code be improved?
Fixing the broken constructor
The FooVideoPlayer class has an issue where it can't safely handle the "not prepared" state. When an instance of FooVideoPlayer is passed around, you don't know if prepare has been called, and calling the wrong function will throw an exception.
Classes or functions that require careful usage can cause bugs. While you can document the precautions, it's ideal to make it impossible to use them incorrectly. In this case, there are a few solutions:
- Execute
prepareduring initialization - Execute
prepareduring the firstplaycall - Make it impossible to call
playbeforeprepare
Option 1: Execute prepare during initialization
The simplest solution is to execute the logic equivalent to prepare in the constructor or initializer.
class FooVideoPlayer(
private val videoUri: Uri,
... // other options
) {
private val preparedValue: PreparedValue
init {
preparedValue = ... // execute `prepare` logic
}
fun play() {
// ... play `videoUri`.
}
}The advantage of this method is that many properties can be read-only. Even properties that are determined during prepare can be declared as val.
However, this approach can lead to unsafe code. If a function called within the initializer reads uninitialized properties, it can cause bugs. Some languages like Swift restrict calling properties or functions before initialization is complete, preventing such bugs. However, this might mean you can't write the logic equivalent to prepare in the initializer. There are various constraints on constructors. For example, in Kotlin, you can't make the constructor itself suspend.
Writing complex logic or logic with significant side effects in the constructor or initializer can cause problems. Therefore, consider making the constructor private and defining a separate factory function to write the logic equivalent to prepare. In the code below, a factory function createInstance is defined within the companion object. (Functions defined within a companion object are equivalent to static methods in Java.)
class FooVideoPlayer private constructor(
private val videoUri: Uri,
..., // other options
private val preparedValue: PreparedValue
) {
fun play() {
// ... play `videoUri`.
}
companion object {
fun createInstance(videoUri: Uri, ...): FooVideoPlayer {
val preparedValue = ... // execute `prepare` logic
return FooVideoPlayer(
videoUri,
...,
preparedValue
)
}
}
}Option 2: Execute prepare during the first play call
Instead of executing the logic equivalent to prepare when the instance is created, you can execute it during the first play call.
class FooVideoPlayer(
private val videoUri: Uri,
... // other options
) {
private var preparedValue: PreparedValue? = null
fun play() {
val preparedValue = prepare()
// ... play `videoUri`.
}
private fun prepare(): PreparedValue {
val existingValue = preparedValue
if (existingValue != null) {
return existingValue
}
val newValue = ... // preparation logic
preparedValue = newValue
return newValue
}
}This method is effective when there's a high chance that play won't be called even if an instance is created, and the cost of prepare is high. However, it has the drawback of requiring properties determined during prepare to be mutable (var). This issue can be mitigated by using mechanisms like Kotlin's lazy, which executes logic on the first access.
class FooVideoPlayer(
private val videoUri: Uri,
... // other options
) {
private val preparedValue: PreparedValue by lazy {
... // preparation logic
}
fun play() {
// ... play `videoUri` by using `preparedValue`
}
}Option 3: Make it impossible to call play before prepare
In statically typed languages, you can separate the types before and after prepare, defining play only after prepare. The idea is to make it impossible to compile if used incorrectly. This method is particularly effective when you want the caller to control the timing of prepare due to reasons like high execution cost. From another perspective, this can be seen as a class-based version of the factory function in Option 1. Unique advantages of this method include the ability to cache prepared instances of FooVideoPlayer and manage "incremental initialization states".
class FooVideoPlayer(
private val videoUri: Uri,
... // other options
) {
fun prepare(): PreparedFooVideoPlayer {
val preparedValue = ... // execute `prepare` logic
return PreparedFooVideoPlayer(
videoUri,
...,
preparedValue
)
}
}
class PreparedFooVideoPlayer(
private val videoUri: Uri,
..., // other options
private val preparedValue: PreparedValue
) {
fun play() {
// ... play `videoUri`.
}
}Summary
Make sure instances that aren't prepared can't be used.
Keywords: initialization, constructor, factory