Skip to content

Commit

Permalink
Kotlinify codebase
Browse files Browse the repository at this point in the history
- Remove unnecessary nullable types
- Replace no-op  method bodies with Unit
- Convert to expression body
- Replace if with when
- Remove braces from 'when' entries
- Remove braces from if statement
- Convert to single line lambda
- oneline if/returns
- Replace 'contains' call with 'in' operator

Following this refactor, it could be great to envision a more "strict" code formatter like ktlint 1.0 (we are currently stuck at 0.48.1)
  • Loading branch information
SimonMarquis committed Nov 16, 2023
1 parent 335a7ec commit 3f966bd
Show file tree
Hide file tree
Showing 46 changed files with 169 additions and 294 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class NavigationTest {
lateinit var topicsRepository: TopicsRepository

private fun AndroidComposeTestRule<*, *>.stringResource(@StringRes resId: Int) =
ReadOnlyProperty<Any?, String> { _, _ -> activity.getString(resId) }
ReadOnlyProperty<Any, String> { _, _ -> activity.getString(resId) }

// The strings used for matching in these tests
private val navigateUp by composeTestRule.stringResource(FeatureForyouR.string.navigate_up)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,7 @@ class MainActivity : ComponentActivity() {
lifecycleScope.launch {
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
viewModel.uiState
.onEach {
uiState = it
}
.onEach { uiState = it }
.collect()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import android.app.Activity
import android.util.Log
import android.view.Window
import androidx.metrics.performance.JankStats
import androidx.metrics.performance.JankStats.OnFrameListener
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
Expand All @@ -29,26 +30,20 @@ import dagger.hilt.android.components.ActivityComponent
@InstallIn(ActivityComponent::class)
object JankStatsModule {
@Provides
fun providesOnFrameListener(): JankStats.OnFrameListener {
return JankStats.OnFrameListener { frameData ->
// Make sure to only log janky frames.
if (frameData.isJank) {
// We're currently logging this but would better report it to a backend.
Log.v("NiA Jank", frameData.toString())
}
fun providesOnFrameListener(): OnFrameListener = OnFrameListener { frameData ->
// Make sure to only log janky frames.
if (frameData.isJank) {
// We're currently logging this but would better report it to a backend.
Log.v("NiA Jank", frameData.toString())
}
}

@Provides
fun providesWindow(activity: Activity): Window {
return activity.window
}
fun providesWindow(activity: Activity): Window = activity.window

@Provides
fun providesJankStats(
window: Window,
frameListener: JankStats.OnFrameListener,
): JankStats {
return JankStats.createAndTrack(window, frameListener)
}
frameListener: OnFrameListener,
): JankStats = JankStats.createAndTrack(window, frameListener)
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ fun NiaApp(
) {
val shouldShowGradientBackground =
appState.currentTopLevelDestination == TopLevelDestination.FOR_YOU
var showSettingsDialog by rememberSaveable {
mutableStateOf(false)
}
var showSettingsDialog by rememberSaveable { mutableStateOf(false) }

NiaBackground {
NiaGradientBackground(
Expand Down Expand Up @@ -126,7 +124,7 @@ fun NiaApp(
appState.topLevelDestinations.forEach { destination ->
val isSelected =
currentDestination.isTopLevelDestinationInHierarchy(destination)
val isUnread = unreadDestinations.contains(destination)
val isUnread = destination in unreadDestinations
item(
selected = isSelected,
icon = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,10 @@ class NiaAppState(
*/
@OptIn(ExperimentalMaterial3AdaptiveNavigationSuiteApi::class)
val navigationSuiteType: NavigationSuiteType
@Composable get() {
return if (windowSize.width > 1240.dp) {
NavigationSuiteType.NavigationDrawer
} else if (windowSize.width >= 600.dp) {
NavigationSuiteType.NavigationRail
} else {
NavigationSuiteType.NavigationBar
}
@Composable get() = when {
windowSize.width > 1240.dp -> NavigationSuiteType.NavigationDrawer
windowSize.width >= 600.dp -> NavigationSuiteType.NavigationRail
else -> NavigationSuiteType.NavigationBar
}

/**
Expand Down Expand Up @@ -180,9 +176,7 @@ class NiaAppState(
}
}

fun navigateToSearch() {
navController.navigateToSearch()
}
fun navigateToSearch() = navController.navigateToSearch()
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ import androidx.test.uiautomator.HasChildrenOp.EXACTLY
fun untilHasChildren(
childCount: Int = 1,
op: HasChildrenOp = AT_LEAST,
): UiObject2Condition<Boolean> {
return object : UiObject2Condition<Boolean>() {
override fun apply(element: UiObject2): Boolean {
return when (op) {
AT_LEAST -> element.childCount >= childCount
EXACTLY -> element.childCount == childCount
AT_MOST -> element.childCount <= childCount
}
}
): UiObject2Condition<Boolean> = object : UiObject2Condition<Boolean>() {
override fun apply(element: UiObject2): Boolean = when (op) {
AT_LEAST -> element.childCount >= childCount
EXACTLY -> element.childCount == childCount
AT_MOST -> element.childCount <= childCount
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.samples.apps.nowinandroid.baselineprofile

import androidx.benchmark.macro.MacrobenchmarkScope
import androidx.benchmark.macro.junit4.BaselineProfileRule
import com.google.samples.apps.nowinandroid.PACKAGE_NAME
import com.google.samples.apps.nowinandroid.startActivityAndAllowNotifications
Expand All @@ -30,11 +31,9 @@ class StartupBaselineProfile {
@get:Rule val baselineProfileRule = BaselineProfileRule()

@Test
fun generate() =
baselineProfileRule.collect(
PACKAGE_NAME,
includeInStartupProfile = true,
) {
startActivityAndAllowNotifications()
}
fun generate() = baselineProfileRule.collect(
PACKAGE_NAME,
includeInStartupProfile = true,
profileBlock = MacrobenchmarkScope::startActivityAndAllowNotifications,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,12 @@ internal abstract class PrintApkLocationTask : DefaultTask() {
fun taskAction() {
val hasFiles = sources.orNull?.any { directory ->
directory.asFileTree.files.any {
it.isFile && it.parentFile.path.contains("build${File.separator}generated").not()
it.isFile && "build${File.separator}generated" !in it.parentFile.path
}
} ?: throw RuntimeException("Cannot check androidTest sources")

// Don't print APK location if there are no androidTest source files
if (!hasFiles) {
return
}
if (!hasFiles) return

val builtArtifacts = builtArtifactsLoader.get().load(apkFolder.get())
?: throw RuntimeException("Cannot load APKs")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,10 @@ import kotlinx.coroutines.flow.onStart

sealed interface Result<out T> {
data class Success<T>(val data: T) : Result<T>
data class Error(val exception: Throwable? = null) : Result<Nothing>
data class Error(val exception: Throwable) : Result<Nothing>
data object Loading : Result<Nothing>
}

fun <T> Flow<T>.asResult(): Flow<Result<T>> {
return this
.map<T, Result<T>> {
Result.Success(it)
}
.onStart { emit(Result.Loading) }
.catch { emit(Result.Error(it)) }
}
fun <T> Flow<T>.asResult(): Flow<Result<T>> = map<T, Result<T>> { Result.Success(it) }
.onStart { emit(Result.Loading) }
.catch { emit(Result.Error(it)) }
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ class DefaultRecentSearchRepository @Inject constructor(

override fun getRecentSearchQueries(limit: Int): Flow<List<RecentSearchQuery>> =
recentSearchQueryDao.getRecentSearchQueryEntities(limit).map { searchQueries ->
searchQueries.map {
it.asExternalModel()
}
searchQueries.map { it.asExternalModel() }
}

override suspend fun clearRecentSearches() = recentSearchQueryDao.clearRecentSearchQueries()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import javax.inject.Inject
* Fake implementation of the [RecentSearchRepository]
*/
class FakeRecentSearchRepository @Inject constructor() : RecentSearchRepository {
override suspend fun insertOrReplaceRecentSearch(searchQuery: String) { /* no-op */ }
override suspend fun insertOrReplaceRecentSearch(searchQuery: String) = Unit

override fun getRecentSearchQueries(limit: Int): Flow<List<RecentSearchQuery>> =
flowOf(emptyList())

override suspend fun clearRecentSearches() { /* no-op */ }
override suspend fun clearRecentSearches() = Unit
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import javax.inject.Inject
*/
class FakeSearchContentsRepository @Inject constructor() : SearchContentsRepository {

override suspend fun populateFtsData() { /* no-op */ }
override suspend fun populateFtsData() = Unit
override fun searchContents(searchQuery: String): Flow<SearchResult> = flowOf()
override fun getSearchContentsCount(): Flow<Int> = flowOf(1)
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ class FakeTopicsRepository @Inject constructor(
)
}.flowOn(ioDispatcher)

override fun getTopic(id: String): Flow<Topic> {
return getTopics().map { it.first { topic -> topic.id == id } }
}
override fun getTopic(id: String): Flow<Topic> = getTopics()
.map { it.first { topic -> topic.id == id } }

override suspend fun syncWith(synchronizer: Synchronizer) = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CompositeUserNewsResourceRepositoryTest {
// Check that only news resources with the given topic id are returned.
assertEquals(
sampleNewsResources
.filter { it.topics.contains(sampleTopic1) }
.filter { sampleTopic1 in it.topics }
.mapToUserNewsResources(emptyUserData),
userNewsResources.first(),
)
Expand All @@ -104,7 +104,7 @@ class CompositeUserNewsResourceRepositoryTest {
// Check that only news resources with the given topic id are returned.
assertEquals(
sampleNewsResources
.filter { it.topics.contains(sampleTopic1) }
.filter { sampleTopic1 in it.topics }
.mapToUserNewsResources(userData),
userNewsResources.first(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ class UserNewsResourceTest {
// Construct the expected FollowableTopic.
val followableTopic = FollowableTopic(
topic = topic,
isFollowed = userData.followedTopics.contains(topic.id),
isFollowed = topic.id in userData.followedTopics,
)
assertTrue(userNewsResource.followableTopics.contains(followableTopic))
}

// Check that the saved flag is set correctly.
assertEquals(
userData.bookmarkedNewsResources.contains(newsResource1.id),
newsResource1.id in userData.bookmarkedNewsResources,
userNewsResource.isSaved,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ val nonPresentInterestsIds = setOf("2")
*/
class TestNewsResourceDao : NewsResourceDao {

private var entitiesStateFlow = MutableStateFlow(
emptyList<NewsResourceEntity>(),
)
private val entitiesStateFlow = MutableStateFlow(emptyList<NewsResourceEntity>())

internal var topicCrossReferences: List<NewsResourceTopicCrossRef> = listOf()

Expand Down Expand Up @@ -131,7 +129,7 @@ class TestNewsResourceDao : NewsResourceDao {
override suspend fun deleteNewsResources(ids: List<String>) {
val idSet = ids.toSet()
entitiesStateFlow.update { entities ->
entities.filterNot { idSet.contains(it.id) }
entities.filterNot { it.id in idSet }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ class TestNiaNetworkDataSource : NiaNetworkDataSource {

override suspend fun getTopics(ids: List<String>?): List<NetworkTopic> =
allTopics.matchIds(
ids = ids,
ids = ids.orEmpty(),
idGetter = NetworkTopic::id,
)

override suspend fun getNewsResources(ids: List<String>?): List<NetworkNewsResource> =
allNewsResources.matchIds(
ids = ids,
ids = ids.orEmpty(),
idGetter = NetworkNewsResource::id,
)

Expand Down Expand Up @@ -91,22 +91,18 @@ class TestNiaNetworkDataSource : NiaNetworkDataSource {
}
}

fun List<NetworkChangeList>.after(version: Int?): List<NetworkChangeList> =
when (version) {
null -> this
else -> this.filter { it.changeListVersion > version }
}
fun List<NetworkChangeList>.after(version: Int?): List<NetworkChangeList> = when (version) {
null -> this
else -> filter { it.changeListVersion > version }
}

/**
* Return items from [this] whose id defined by [idGetter] is in [ids] if [ids] is not null
*/
private fun <T> List<T>.matchIds(
ids: List<String>?,
ids: List<String>,
idGetter: (T) -> String,
) = when (ids) {
null -> this
else -> ids.toSet().let { idSet -> this.filter { idSet.contains(idGetter(it)) } }
}
) = ids.toSet().let { idSet -> filter { idGetter(it) in idSet } }

/**
* Maps items to a change list where the change list version is denoted by the index of each item.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,15 @@ import kotlinx.coroutines.flow.update
*/
class TestTopicDao : TopicDao {

private var entitiesStateFlow = MutableStateFlow(
emptyList<TopicEntity>(),
)
private val entitiesStateFlow = MutableStateFlow(emptyList<TopicEntity>())

override fun getTopicEntity(topicId: String): Flow<TopicEntity> {
override fun getTopicEntity(topicId: String): Flow<TopicEntity> =
throw NotImplementedError("Unused in tests")
}

override fun getTopicEntities(): Flow<List<TopicEntity>> =
entitiesStateFlow
override fun getTopicEntities(): Flow<List<TopicEntity>> = entitiesStateFlow

override fun getTopicEntities(ids: Set<String>): Flow<List<TopicEntity>> =
getTopicEntities()
.map { topics -> topics.filter { it.id in ids } }
getTopicEntities().map { topics -> topics.filter { it.id in ids } }

override suspend fun getOneOffTopicEntities(): List<TopicEntity> = emptyList()

Expand All @@ -55,15 +50,11 @@ class TestTopicDao : TopicDao {

override suspend fun upsertTopics(entities: List<TopicEntity>) {
// Overwrite old values with new values
entitiesStateFlow.update { oldValues ->
(entities + oldValues).distinctBy(TopicEntity::id)
}
entitiesStateFlow.update { oldValues -> (entities + oldValues).distinctBy(TopicEntity::id) }
}

override suspend fun deleteTopics(ids: List<String>) {
val idSet = ids.toSet()
entitiesStateFlow.update { entities ->
entities.filterNot { idSet.contains(it.id) }
}
entitiesStateFlow.update { entities -> entities.filterNot { it.id in idSet } }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ object ListToMapMigration : DataMigration<UserPreferences> {
hasDoneListToMapMigration = true
}

override suspend fun shouldMigrate(currentData: UserPreferences): Boolean {
return !currentData.hasDoneListToMapMigration
}
override suspend fun shouldMigrate(currentData: UserPreferences): Boolean =
!currentData.hasDoneListToMapMigration
}
Loading

0 comments on commit 3f966bd

Please sign in to comment.