The original article was published on August 22, 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.
Contact before work
The following FooMediaPlayer
is a class for playing media. The main function is play
, where media is loaded by initMedia
and the actual playback process is performed by playMedia
. The playback state is exposed by playingState
.
class FooMediaPlayer {
private var playingMediaMetaData: MediaMetaData? = null
private var playingMedia: Media? = null
var playingState: PlayingState = PlayingState.None
private set
fun play(mediaMetaData: MediaMetaData) {
playingMediaMetaData = mediaMetaData
initMedia()
playMedia()
}
private fun initMedia() {
val mediaPath = playingMediaMetaData?.filePath
if (mediaPath != null && !isValidMediaFile(mediaPath)) {
playingMedia = null
return
}
...
playingMedia = ...
}
private fun playMedia() {
val media = playingMedia
if (media == null) {
playingState = PlayingState.None
return
}
...
playingState = PlayingState(media, ...)
...
}
}
Is there anything that should be improved in this code?
Dependencies that can "contact"
There is a constraint in initMedia
and playMedia
that initMedia
must be called first. However, changing the call order will still compile, resulting in a runtime error. In such cases, it is good to explicitly state the constraint using arguments. In other words, it is good to design in a way that enforces the correct order.
In the following implementation, playMedia
requires the return value of fetchMedia
as an argument, making it less likely to call fetchMedia
and playMedia
in the wrong order.
class FooMediaPlayer {
var playingState: PlayingState = PlayingState.None
private set
fun play(mediaMetaData: MediaMetaData) {
val media = fetchMedia(mediaMetaData)
playingState = playMedia(media)
}
private fun fetchMedia(metaData: MediaMetaData): Media? {
if (!isValidMediaFile(metaData.filePath)) {
return null
}
val media = ...
...
return media
}
private fun playMedia(media: Media?): PlayingState {
if (media == null) {
return PlayingState.None
}
...
return PlayingState(media, ...)
}
}
From the perspective of cohesion and coupling
Using arguments to clarify order also means stronger cohesion and weaker coupling.
Cohesion: The pre-improvement play
consists of multiple actions with a fixed order, making it a type of procedural strength. In contrast, the post-improvement version has a relationship where information is passed through return values and arguments, resulting in communicational strength, which is stronger cohesion.
Coupling: In the pre-improvement initMedia
and playMedia
, information is shared through instance properties, resulting in common coupling at the class scope. In contrast, the post-improvement code passes structured values as arguments, resulting in stamp coupling, which is weaker coupling. For more on coupling, please also refer to Code readability: Session 6 (ver. 2, En).
In a nutshell
Effectively use arguments and return values to clarify the order of operations.
Keywords: implicit dependency
, function flow
, strength