Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detekt - Refactor and optimize code across multiple modules #240

Merged
merged 3 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ dependencies {

// Projects
implementation(projects.core.designSystem)
implementation(projects.core.domain)
implementation(projects.core.network)
implementation(projects.core.utils)
implementation(projects.feature.sydneyTrains.database.api)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ import xyz.ksharma.krail.trip.planner.ui.navigation.tripPlannerDestinations
* from Slack, but that would also mean refactoring to use MVP instead of MVVM.
*/
@Composable
fun KrailNavHost() {
fun KrailNavHost(modifier: Modifier = Modifier) {
val navController = rememberNavController()

NavHost(
navController = navController,
startDestination = SplashScreen,
modifier = Modifier.fillMaxSize(),
modifier = modifier.fillMaxSize(),
) {
tripPlannerDestinations(navController = navController)

Expand Down Expand Up @@ -122,7 +122,7 @@ fun AnimatedKrailLogo(modifier: Modifier = Modifier) {
}

@Composable
fun AnimatedLetter(letter: String, animationStarted: Boolean) {
fun AnimatedLetter(letter: String, animationStarted: Boolean, modifier: Modifier = Modifier) {
val infiniteTransition = rememberInfiniteTransition(label = "animeAnimation")

// Scale animation with anticipation and squash/stretch
Expand Down Expand Up @@ -156,7 +156,7 @@ fun AnimatedLetter(letter: String, animationStarted: Boolean) {
width = 8f, // Adjust stroke width for desired thickness
),
),
modifier = Modifier
modifier = modifier
.graphicsLayer {
scaleX = letterScale
scaleY = letterScale
Expand Down
12 changes: 7 additions & 5 deletions config/detekt/detekt.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ Compose:
DefaultsVisibility:
active: true
LambdaParameterInRestartableEffect:
active: true
active: false
# -- You can optionally have a list of types to be treated as lambdas (e.g. typedefs or fun interfaces not picked up automatically)
# treatAsLambda: MyLambdaType
Material2:
active: false # Opt-in, disabled by default. Turn on if you want to disallow Material 2 usages.
active: true # Opt-in, disabled by default. Turn on if you want to disallow Material 2 usages.
# -- You can optionally allow parts of it, if you are in the middle of a migration.
# allowedFromM2: icons.Icons,TopAppBar
ModifierClickableOrder:
Expand Down Expand Up @@ -127,6 +127,7 @@ complexity:
ignoreAnnotation: true
excludeStringsWithLessThan5Characters: true
ignoreStringsRegex: '$^'
ignoreAnnotated: ['Preview', 'PreviewLightDark', 'PreviewComponent']
ComplexInterface:
active: false
threshold: 10
Expand All @@ -141,14 +142,15 @@ complexity:
MethodOverloading:
active: true
TooManyFunctions:
active: false
ignoreAnnotatedFunctions: ['Composable']
excludes: ['**/test/**', '**/functionalTest/**']
LongMethod:
active: true
threshold: 60
ignoreAnnotated: ['Composable']
LongParameterList:
active: true
active: false
ignoreAnnotated: ['Composable', 'Test', 'GET', 'POST']

coroutines:
Expand Down Expand Up @@ -210,7 +212,7 @@ naming:
FunctionNaming:
active: true
excludes: []
ignoreAnnotated: ['Test', 'ParameterizedTest', 'RepeatedTest', 'TestFactory']
ignoreAnnotated: ['Test', 'ParameterizedTest', 'RepeatedTest', 'TestFactory', 'Composable']
TopLevelPropertyNaming:
constantPattern: '[a-z][_A-Za-z0-9]*|[A-Z][_A-Z0-9]*'
InvalidPackageDeclaration:
Expand Down Expand Up @@ -264,7 +266,7 @@ style:
CanBeNonNullable:
active: true
CascadingCallWrapping:
active: true
active: false
ClassOrdering:
active: true
CollapsibleIfStatements:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fun TextField(
textStyle: TextStyle? = null,
readOnly: Boolean = false,
imeAction: ImeAction = ImeAction.Default,
onTextChanged: (CharSequence) -> Unit = {},
onTextChange: (CharSequence) -> Unit = {},
) {
val interactionSource = remember { MutableInteractionSource() }
val isFocused by interactionSource.collectIsFocusedAsState()
Expand All @@ -66,7 +66,7 @@ fun TextField(
)

LaunchedEffect(textFieldState.text) {
onTextChanged(textFieldState.text)
onTextChange(textFieldState.text)
}

CompositionLocalProvider(
Expand Down
16 changes: 0 additions & 16 deletions core/domain/build.gradle.kts

This file was deleted.

Binary file removed core/domain/src/main/kotlin/xyz/.DS_Store
Binary file not shown.
Binary file not shown.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import java.util.concurrent.TimeUnit
@InstallIn(SingletonComponent::class)
object NetworkModule {

const val BASE_URL = "https://api.transport.nsw.gov.au"

@Provides
fun provideOkHttpClient(): OkHttpClient {
val okhttpBuilder = OkHttpClient.Builder()
Expand Down Expand Up @@ -55,6 +57,4 @@ object NetworkModule {
.build()
return retrofit
}

const val BASE_URL = "https://api.transport.nsw.gov.au"
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ interface StopTimesStore {
dropOffType: Int?,
)

/**
* Removes all stop times from the database.
*/
suspend fun clearStopTimes()

/**
* Adds a batch of stop times to the database.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,4 @@ class RealStopTimesStore @Inject constructor(
}

override suspend fun stopTimesSize(): Long = getSydneyTrainsDb().stoptimesQueries.sizeOfStopTimes().executeAsOne()

override suspend fun clearStopTimes() {
TODO("Not yet implemented")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ data class StopItem(
) : Serializable {
fun toJsonString() = Json.encodeToString(serializer(), this)

@Suppress("ConstPropertyName")
companion object {
private const val serialVersionUID: Long = 1L

fun fromJsonString(json: String) =
kotlin.runCatching { Json.decodeFromString(serializer(), json) }.getOrNull()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@ import androidx.compose.ui.graphics.Color
*
* @return A Compose Color object representing the provided hex color code.
*/
internal fun String.hexToComposeColor(): Color =
if (this.isValidHexColorCode()) {
Color(android.graphics.Color.parseColor(this))
} else {
throw IllegalArgumentException("Invalid hex color code: $this")
}
internal fun String.hexToComposeColor(): Color {
require(this.isValidHexColorCode()) { "Invalid hex color code: $this" }
return Color(android.graphics.Color.parseColor(this))
}

/**
* Checks if a string is a valid hexadecimal color code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ fun JourneyCard(
originTime: String,
destinationTime: String,
totalTravelTime: String,
platformNumber: Char? = null,
isWheelchairAccessible: Boolean,
transportModeList: ImmutableList<TransportMode>? = null,
onClick: () -> Unit,
modifier: Modifier = Modifier,
platformNumber: Char? = null,
transportModeList: ImmutableList<TransportMode>? = null,
) {
val onSurface: Color = KrailTheme.colors.onSurface
val borderColors = remember(transportModeList) { transportModeList.toColors(onSurface) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ fun JourneyDetailCard(
timeToDeparture: String,
platformNumber: Char,
totalTravelTime: String,
transportModeList: ImmutableList<TransportMode>? = null, // TODO cannot be null. Need to handle this
legList: ImmutableList<TimeTableState.JourneyCardInfo.Leg>,
onClick: () -> Unit,
modifier: Modifier = Modifier,
transportModeList: ImmutableList<TransportMode>? = null, // TODO cannot be null. Need to handle this
) {
val density = LocalDensity.current
// todo can be reusable logic for consistent icon size
Expand Down Expand Up @@ -201,8 +201,16 @@ fun JourneyDetailCard(
JourneyLeg(
transportModeLine = leg.transportModeLine,
stopsInfo = leg.stopsInfo,
departureTime = if (index == legList.lastIndex) leg.stops.last().time else leg.stops.first().time,
stopName = if (index == legList.lastIndex) leg.stops.last().name else leg.stops.first().name,
departureTime = if (index == legList.lastIndex) {
leg.stops.last().time
} else {
leg.stops.first().time
},
stopName = if (index == legList.lastIndex) {
leg.stops.last().name
} else {
leg.stops.first().name
},
isWheelchairAccessible = false,
modifier = Modifier.padding(start = 8.dp, end = 8.dp),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ fun JourneyLeg(
stopsInfo: String,
departureTime: String,
stopName: String,
isWheelchairAccessible: Boolean = false,
modifier: Modifier = Modifier,
isWheelchairAccessible: Boolean = false,
) {
Column(
modifier = modifier
Expand Down Expand Up @@ -68,7 +68,11 @@ fun JourneyLeg(
text = stopsInfo,
modifier = Modifier.align(Alignment.CenterVertically),
style = KrailTheme.typography.title,
color = if (isSystemInDarkTheme()) KrailTheme.colors.onSurface else transportModeLine.transportMode.colorCode.hexToComposeColor(),
color = if (isSystemInDarkTheme()) {
KrailTheme.colors.onSurface
} else {
transportModeLine.transportMode.colorCode.hexToComposeColor()
},
)
}
FlowRow(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ fun TransportModeInfo(
backgroundColor: Color,
badgeColor: Color,
badgeText: String,
borderEnabled: Boolean = false,
modifier: Modifier = Modifier,
borderEnabled: Boolean = false,
) {
Row(modifier = modifier, horizontalArrangement = Arrangement.spacedBy(8.dp)) {
TransportModeIcon(letter = letter, backgroundColor = backgroundColor, borderEnabled = borderEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import xyz.ksharma.krail.trip.planner.ui.timetable.timeTableDestination
* It contains all the screens in the feature Trip Planner.
*/
fun NavGraphBuilder.tripPlannerDestinations(
navController: NavHostController, // TODO - do not wanna add NavController here, but moving all callbacks to app module is not scaleable.
// TODO - do not wanna add NavController here, but moving all callbacks to app module is not scaleable.
navController: NavHostController,
) {
navigation<TripPlannerNavRoute>(startDestination = SavedTripsRoute) {
savedTripsDestination(navController)

searchStopDestination(navController)

timeTableDestination(navController)
timeTableDestination()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavGraphBuilder
import androidx.navigation.NavHostController
import androidx.navigation.compose.composable
Expand All @@ -20,8 +18,8 @@ import xyz.ksharma.krail.trip.planner.ui.state.searchstop.model.StopItem.Compani

internal fun NavGraphBuilder.savedTripsDestination(navController: NavHostController) {
composable<SavedTripsRoute> { backStackEntry ->
val viewModel = hiltViewModel<SavedTripsViewModel>()
val savedTripState by viewModel.uiState.collectAsStateWithLifecycle()
// val viewModel = hiltViewModel<SavedTripsViewModel>()
// val savedTripState by viewModel.uiState.collectAsStateWithLifecycle()

val fromArg: String? =
backStackEntry.savedStateHandle.get<String>(SearchStopFieldType.FROM.key)
Expand All @@ -47,7 +45,6 @@ internal fun NavGraphBuilder.savedTripsDestination(navController: NavHostControl
}

SavedTripsScreen(
savedTripsState = savedTripState,
fromStopItem = fromStopItem,
toStopItem = toStopItem,
fromButtonClick = {
Expand Down Expand Up @@ -84,8 +81,6 @@ internal fun NavGraphBuilder.savedTripsDestination(navController: NavHostControl
Timber.e("Select both stops")
}
},
) { event ->
viewModel.onEvent(event)
}
)
}
}
Loading