althaf
11/08/2023, 3:43 PMoverride suspend fun saveToDeviceRegistry(req: SaveToDeviceRegistryReq): Flow<RepositoryState<Unit>> =
flow<RepositoryState<Unit>> {
searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)?.let {
it.categoryName = req.deviceProfile.categoryName
it.categoryId = req.deviceProfile.categoryId
it.vendor = req.deviceProfile.vendor
it.model = req.deviceProfile.model
it.mac = req.deviceProfile.address
deviceRegistryBox.put(it)
emit(RepositoryState.SuccessState(Unit))
} ?: kotlin.run {
deviceRegistryBox.put(
DeviceRegistryEntity(
categoryName = req.deviceProfile.categoryName,
categoryId = req.deviceProfile.categoryId,
vendor = req.deviceProfile.vendor,
model = req.deviceProfile.model,
mac = req.deviceProfile.address,
)
)
emit(RepositoryState.SuccessState(Unit))
}
}.flowOn(dispatcher)
Is there syntax sugar to write a better code in Kotlin with the above snippet,Jacob
11/08/2023, 3:47 PMalthaf
11/08/2023, 3:49 PMalthaf
11/08/2023, 3:50 PMJacob
11/08/2023, 3:53 PMval registryEntity = searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)?.let {
it.categoryName = req.deviceProfile.categoryName
it.categoryId = req.deviceProfile.categoryId
it.vendor = req.deviceProfile.vendor
it.model = req.deviceProfile.model
it.mac = req.deviceProfile.address
} ?:
DeviceRegistryEntity(
categoryName = req.deviceProfile.categoryName,
categoryId = req.deviceProfile.categoryId,
vendor = req.deviceProfile.vendor,
model = req.deviceProfile.model,
mac = req.deviceProfile.address,
)
and then call put on the registryEntityascii
11/08/2023, 3:54 PMit.*
assignments into a different function and do deviceRegistryBox.put(separatefn(it))
.
2. You don't need to do kotlin.run
. Simply run
is enough.
3. As this flow always returns Success, you don't need the last run block. Have the emit
as the last statement.
searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId).let {
deviceRegistryBox.put(
// `updateFields` is the separate function that does all the `it.*` assignments.
if (it != null) updateFields(it) else DeviceRegistryEntity(…)
)
}
emit(Success)
ascii
11/08/2023, 3:55 PMDeviceRegistryEntity(…)
could also be a separate fn with req
as an input parameter. Would help with unit tests.David Kubecka
11/08/2023, 3:57 PMJacob
11/08/2023, 3:59 PMval registryEntity = searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)?.apply {
updateRegistryEntity(req.deviceProfile)
} ?: newRegistryEntity(req.deviceProfile)
ascii
11/08/2023, 4:00 PMJacob
11/08/2023, 4:00 PMDavid Kubecka
11/08/2023, 4:03 PMDeviceRegistryEntity
and then use updateRegistryEntity
in both cases. That's not ideal and I wonder whether there's another possible solution for the deduplication.Jacob
11/08/2023, 4:07 PMval registryEntity = searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId).orElseNew().apply{update(req.deviceProfile)}
fun DeviceRegistryEntity?.orElseNew() = this ?: DeviceRegistryEntity()
fun DeviceRegistryEntity.update(deviceProfile: DeviceProfile){
...
}
David Kubecka
11/08/2023, 4:08 PMDeviceRegistryEntity
have default (most probably null) values.Youssef Shoaib [MOD]
11/08/2023, 4:15 PMval entity = DeviceRegistryEntity(...)
deviceRegistryBox.put(searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)?.mergeWith(entity) ?: entity)
emit(...)
fun searchDeviceRegistryByCategoryId(...): ThoseExtraFields
fun ThoseExtraFields.mergeWith(entity: DeviceRegistryEntity) = ...
The key here is that the search shouldn't be returning a DeviceRegistryEntity if it can never return a valid one. Instead, the search should have its own abstraction.
If the search is really tightly coupled, you might instead want to do:
val entity = DeviceRegistryEntity(...)
deviceRegistryBox.put(searchDeviceRegistryByCategoryId(entity))
emit(...)
fun searchDeviceRegistryByCategoryId(entity: DeviceRegistryEntity): DeviceRegistryEntity
David Kubecka
11/08/2023, 4:30 PMdataClassInstance.copy(a = 1, b = 2)
and in the create branch
DataClass(a = 1, b = 2)
Jacob
11/08/2023, 4:33 PMDataClass().copy(a = 1, b = 2)
and the duplication is removedDavid Kubecka
11/08/2023, 4:34 PMDavid Kubecka
11/08/2023, 4:36 PMalthaf
11/08/2023, 5:38 PMalthaf
11/08/2023, 5:39 PMoverride suspend fun saveToDeviceRegistry(req: SaveToDeviceRegistryReq): Flow<RepositoryState<Unit>> =
flow<RepositoryState<Unit>> {
req.deviceProfile.apply {
deviceRegistryBox.put(searchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)?.let {
this.toDeviceRegistryEntity(it.id))
} ?: this.toDeviceRegistryEntity(0))
emit(RepositoryState.SuccessState(Unit))
}
}.flowOn(dispatcher)
althaf
11/08/2023, 5:39 PMfun DeviceProfile.toDeviceRegistryEntity(id: Long): DeviceRegistryEntity =
DeviceRegistryEntity(
id = id,
categoryName = categoryName,
categoryId = categoryId,
vendor = vendor,
model = model,
mac = address
)
althaf
11/08/2023, 5:41 PMYoussef Shoaib [MOD]
11/08/2023, 5:46 PMoverride suspend fun saveToDeviceRegistry(req: SaveToDeviceRegistryReq): Flow<RepositoryState<Unit>> = with(req.deviceProfile) {
flow {
deviceRegistryBox.put(toDeviceRegistryEntity(searchDeviceRegistryByCategoryId(categoryId)?.id ?: 0))
emit(RepositoryState.SuccessState(Unit))
}.flowOn(dispatcher)
}
Has the same logic but is immensely clearer IMO
or:
override suspend fun saveToDeviceRegistry(req: SaveToDeviceRegistryReq): Flow<RepositoryState<Unit>> = with(req.deviceProfile) {
flow {
val id = searchDeviceRegistryByCategoryId(categoryId)?.id ?: 0
deviceRegistryBox.put(toDeviceRegistryEntity(id))
emit(RepositoryState.SuccessState(Unit))
}.flowOn(dispatcher)
}
Jacob
11/08/2023, 5:46 PMapply
to a with
. apply changes the returned value to this
from the far more typical return type of the block. I wouldn’t use it unless I was leveraging the return typeJacob
11/08/2023, 5:48 PMsearchDeviceRegistryByCategoryId(req.deviceProfile.categoryId)
should be using this.categoryIdalthaf
11/08/2023, 5:49 PMoverride suspend fun saveToDeviceRegistry(req: SaveToDeviceRegistryReq): Flow<RepositoryState<Unit>> =
flow<RepositoryState<Unit>> {
with(req.deviceProfile) {
deviceRegistryBox.put(searchDeviceRegistryByCategoryId(categoryId)?.let {
toDeviceRegistryEntity(it.id)
} ?: toDeviceRegistryEntity(0))
emit(RepositoryState.SuccessState(Unit))
}
}.flowOn(dispatcher)
althaf
11/08/2023, 5:49 PMJacob
11/08/2023, 5:49 PMalthaf
11/08/2023, 5:51 PMDaniel Pitts
11/08/2023, 9:15 PMreq.deviceProfile
is what you're actually using, so perhaps that should be the parameter instead.