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.
Accumulating dependencies
The following LatestNewsSnippetUseCase is a class that provides a news article snippet as a data model NewsSnippet. This data model is created by combining repository data and formatting strings.
class LatestNewsSnippetUseCase(
private val locale: Locale,
private val articleRepository: NewsArticleRepository,
private val sourceRepository: NewsSourceRepository,
private val stringTruncator: StringTruncator,
private val timeFormatter: TimeTextFormatter = TimeTextFormatterImpl(locale),
private val modelFactory: (title: String, content: String, dateText: String, source: String) -> NewsSnippet =
::NewsSnippet
) {
fun getLatestSnippet(): NewsSnippet {
val article = articleRepository.getLatestArticle()
val articleText = stringTruncator.truncate(article.contextText, ARTICLE_TEXT_LENGTH, locale)
val dateText = timeFormatter.toShortMonthDayText(article.timestampInMillis)
val sourceName = sourceRepository.getSource(article.sourceId).shortName
return modelFactory(article.title, articleText, dateText, sourceName)
}
companion object {
private const val ARTICLE_TEXT_LENGTH = 280
}
}There are two repositories, NewsArticleRepository and NewsSourceRepository, which are assumed to be complex classes (e.g., dependent on the network). There are two string formatters, StringTruncator and TimeTextFormatter, but their implementations are assumed to be simple. (In reality, handling strings tends to be complex, but for explanation purposes, it is simplified.)
The constructor parameter modelFactory indicates a function that creates an instance of NewSnippet. The default argument is defined as a reference to the constructor of NewSnippet, ::NewSnippet. In other words, the default argument behavior is that calling modelFactory(...) is the same as calling the constructor NewSnippet(...).
Here, StringTruncator, TimeTextFormatter, and NewsSnippet are defined as follows:
interface StringTruncator {
fun truncate(string: String, length: Int, locale: Locale, suffix: String = "…" /* U+2026 */): String
}
interface TimeTextFormatter {
fun toShortMonthDayText(millis: Long): String
}
class NewsSnippet(val title: String, val content: String, val dateText: String, val source: String)Furthermore, the constructor of the StringTruncator implementation StringTruncatorImpl is assumed to take no arguments.
Is there anything that can be improved in this code?
The raison d'être of injection
In short, there is no need to allow modelFactory to be passed from the outside. Also, for StringTruncator and TimeTextFormatter, if they do not depend on singletons, it is unnecessary to separate the interface and pass the implementation from the outside (dependency injection). The only things that should be passed from the outside in this code are values that change depending on the environment (Locale) and complex classes (repositories). To simplify the code, it can be rewritten as follows:
modelFactory: Call the constructor ofNewsSnippetdirectly.stringTruncator/timeFormatter: Keep the instance as a normalprivateproperty.- Consider integrating the interface and implementation.
- If
StringTruncatoris stateless, it can be changed to anobject.
class LatestNewsSnippetUseCase(
private val locale: Locale,
private val articleRepository: NewsArticleRepository,
private val sourceRepository: NewsSourceRepository
) {
private val stringTruncator: StringTruncator = StringTruncatorImpl()
private val timeFormatter: TimeTextFormatter = TimeTextFormatterImpl(locale)
suspend fun getLatestSnippet(): NewsSnippet {
val article = articleRepository.getLatestArticle()
val articleText = stringTruncator.truncate(article.contextText, ARTICLE_TEXT_LENGTH, locale)
val dateText = timeFormatter.toShortMonthDayText(article.timestampInMillis)
val sourceName = sourceRepository.getSource(article.sourceId).shortName
return NewsSnippet(article.title, articleText, dateText, sourceName)
}
companion object {
private const val ARTICLE_TEXT_LENGTH = 280
}
}When using dependency injection, the purpose must be clear. The purposes of injection include the following:
- Managing the scope and lifecycle of dependencies: To share objects (e.g., sharing state, separating cross-cutting concerns) or to use objects with a longer lifespan than themselves.
- Dependency inversion: To resolve circular dependencies in modules or to follow the dependency direction defined by the architecture.
- Switching implementations: To switch functions based on settings or to replace with implementations for testing, debugging, or verification.
- Separating implementations: To speed up builds or to provide proprietary libraries.
If these are not necessary, for example, for referentially transparent utility functions or simple model classes, there is almost no need for injection. Unnecessary injection can lead to the following issues:
Unnecessary and implicit dependencies: When injecting through a constructor, you need to check the caller of the constructor to track the behavior of the dependency. If the constructor is called in various places, it will take effort to check all the callers. Even if injection is done in other ways, the separation of interfaces increases the effort to trace implementations.
Increased responsibility of the caller: When injecting through constructors or setters, the caller must resolve the dependencies. Especially if dependency resolution is transferred in a chain, all dependencies will gather in the main class. (This issue can be mitigated by secondary constructors, factory functions, default arguments, DI containers, etc.)
Breaking the correlation of values: Even if there is a constraint that "a common value should be used across multiple objects," that constraint can be broken by value injection. In the example of the UseCase above, it is implicitly expected that the Locale used in StringTruncator.truncate and TimeTextFormatter is the same. However, in reality, it is possible to pass a different locale like LatestNewsSnippetUseCase(locale1, ..., TimeTextFormatterImpl(locale2), ...). Especially if dependency resolution is done elsewhere, it is easy to overlook if different values are passed. Also, tests to verify that common values are used can become more difficult to write.
In a nutshell
When performing dependency injection, clarify its purpose.
Keywords: dependency injection, dependency explicitness, constructor parameter