https://kotlinlang.org logo
#codereview
Title
# codereview
t

therealbluepandabear

03/02/2021, 5:20 AM
How is this code I did with DI. How could I improve this?
Copy code
fun main() {
    val capitalsService: CapitalsService = CapitalsServiceImpl(CapitalsRepositoryImpl())
    capitalsService.getCapitals()
}

interface CapitalsService {
    fun getCapitals(): List<Capital>
}

class CapitalsServiceImpl(private val capitalsRepository: CapitalsRepository) : CapitalsService {
    override fun getCapitals(): List<Capital> = capitalsRepository.getCapitals()
}

interface CapitalsRepository {
    fun getCapitals(): List<Capital>
}

class CapitalsRepositoryImpl : CapitalsRepository {
    override fun getCapitals() = listOf(Capital("Paris", Country("France", "Europe", 67399000), 2175601))
}

data class Country(val countryName: String?, val continent: String?, val population: Int?)

data class Capital(val cityName: String?, val capitalOf: Country?, val population: Int?)
m

Matteo Mirk

03/02/2021, 8:53 AM
Looks good in general, you performed manual DI which I personally favor in place of DI containers. What’s the purpose of the service? Right now it does the same as the repository, does it have a special purpose? I apologize if the code is just for sample purposes. Also, why do all your data classes have nullable properties? Does it make sense in your domain to have a country or capital without any data inside?
j

Joel

03/02/2021, 3:25 PM
s/fun getCapitals(): List<Capital>/val capitals: List<Capital>
Part of the fun of Kotlin is that getters and setters are defined automagically. Your interfaces and objects have
capitals
, and can be implemented as
val capitals = listOf(...)
or`val capitals: List<Capital> get() = ...` if necessary.
I personally use
Iterable
over
List
whenever possible, but that may change depending on context and whatever you're trying to achieve.
d

Dmitry Kandalov

03/05/2021, 9:26 AM
Using interfaces in this examples looks like an overkill since they have only one implementation. Naming classes with “Impl” postfix is a smell.
2
j

Joel

03/05/2021, 3:04 PM
Using an interface for a single implementation is useful in some cases, like if you're writing a library. @Dmitry Kandalov what do you use instead of
Impl
postfix? I've seen
Default
prefix and
IThing
as an interface name, but that feels odd.
m

Matteo Mirk

03/05/2021, 3:20 PM
“Default” or “Base” could do in some cases, but I prefer to find domain-related names for interfaces and their implementations. For example interface
Capitals
-> class:
ConstantCapitals, HttpCapitals, FileCapitals
etc.
👍 2
d

Dmitry Kandalov

03/05/2021, 3:48 PM
👍 domain or just descriptive names are the best option. In the example above maybe
StubCapitalsRepository
,
InMemoryCapitalsRepository
or
DbCapitalsRepository
if it actually reads data from database. Another trick in Kotlin is to define
invoke()
function in the companion object of interface and put default implementation there so you don’t need to come up with explicit name:
Copy code
interface Foo {
    fun bar()
    
    companion object {
        operator fun invoke() = object : Foo {
            override fun bar() = 123
        }
    }
}
👍 2
2 Views