Best Practices

Summary

The point of this doc is to share some best practices and processes within the team so that we are all on the same page regarding the points listed.

Target Audience

Dev team :)

IDE

Use the Code Style settings provided by the project. Note that the codestyle has been checked into git.
Use the Run Configurations checked into project (via the Store as project file checkbox). There are a ton checked into the project and available for use (ss might not represent the latest set of Run Configurations).

Run Configurations Explained

Relative Importance Key

High - Task expected to be used very frequently.
Medium - Task expected to be used occasionally.
Low - Task available but not expected to be used very frequently.

Run Configuration	Relative Importance?	Typical execution cadence?	Description	Use Case
`bundleInternalDebug`	Low		Executes the gradle task to create an `internalDebug` variant app bundle	Validate app bundle built locally.
`bundleProductionRelease CI test w/ fake data`	Low		Executes the gradle task to create a `productionRelease` variant app bundle as if run on CI using fake data specified in the Run Configuration environment variables	Validate app bundle renaming logic as if built by CI.
`bundleProductionRelease non-CI test w/ fake data`	Low		Executes the gradle task to create a `productionRelease` variant app bundle as if NOT run on CI using fake data specified in the Run Configuration environment variables	Validate app bundle renaming logic as if built locally.
`detekt`	Low		Executes the detekt gradle task to perform static analysis. Build will fail on detekt failure.	Run locally to ensure build won't fail on CI. Remediate issues found until task executes successfully.
`ktlintCheckDetekt`	Low		Executes both ktlint check and detekt gradle tasks.	Run locally to ensure build won't fail on CI. Remediate issues found until task executes successfully.
`testInternalDebugCombinedUnitTestCoverage`	Medium	Viewing/increasing unit test coverage	Executes a custom jacoco gradle task that generates a combined jacoco report of both `:app` and `:data` modules (in addition to the separate reports per module)	When desiring to increase unit test coverage, you can execute this task followed by `Open Jacoco Report - internalDebug` to view existing coverage, write new unit tests to increase coverage, and repeat the process to view/validate the coverage increases.
`testInternalDebugUnitTestCoverage`	Low		Executes a custom jacoco gradle task that generates separate jacoco reports per module. Prefer executing `testInternalDebugCombinedUnitTestCoverage` instead.	Prefer executing `testInternalDebugCombinedUnitTestCoverage` instead.
`testInternalDebugUnitTestCoverageVerification`	Low		Executes a custom jacoco gradle task that fails the build when the minimum code coverage threshold is NOT met (per module).	Run locally to ensure that the build won't fail due to minimum code coverage thresholds NOT being met.
`ktlintFormat`	High	Prior to pushing to remote	Executes ktlint format gradle task to automatically fix most ktlint errors. Manual error resolution might still be required.	Run locally prior to pushing local commits to the remote git repository to automatically fix most ktlint errors. Remediate issues found until task executes successfully.
`lintInternalDebug`	Low		Executes Android lint gradle task. Build will fail on lint error.	Run locally to ensure build won't fail on CI. Remediate issues found until task executes successfully.
`dependencyUpdates`	High	When updating project dependencies	Executes dependency updates gradle task which reports which dependencies have updates available.	Run to see what dependency updates are available when desiring to update dependencies.
`:app - testInternalDebugUnitTest`	Low		Executes all `:app` module unit tests	Run to validate success/failure of all `:app` module unit tests with AS unit test run UI.
`:data - testInternalDebugUnitTest`	Low		Executes all `:data` module unit tests	Run to validate success/failure of all `:data` module unit tests with AS unit test run UI.
`ci - internalDebug (minus clean) - dev build`	High	Prior to pushing to remote	Executes all gradle tasks that would be run on CI for the `internalDebug` variant. If this task succeeds, there is a 99.9% chance the build will succeed on CI.	Run locally to ensure build won't fail on CI prior to pushing local commits to the remote git repository. Remediate issues found until task executes successfully.
`ci - internalRelease (minus clean) - qa build`	Low		Executes all gradle tasks that would be run on CI for the `internalRelease` variant. If this task succeeds, there is a 99.9% chance the build will succeed on CI.	Run locally to ensure build won't fail on CI prior to pushing local commits to the remote git repository. Remediate issues found until task executes successfully.
`ci - productionRelease (minus clean) - CI store build w/ fake data`	Low		Executes all gradle tasks that would be run on CI for the `productionRelease` variant. If this task succeeds, there is a 99.9% chance the build will succeed on CI.	Run locally to ensure build won't fail on CI prior to pushing local commits to the remote git repository. Remediate issues found until task executes successfully.
`ktlintCheck`	Low		Executes ktlint check task to show any ktlint code style/formatting errors.	Run locally to ensure build won't fail on CI. Remediate issues found until task executes successfully.
`Open Jacoco Report - internalDebug`	Medium	Viewing/increasing unit test coverage	Executes shell script to open the `internalDebug` variant HTML jacoco reports (unified, `:app`, and `:data`) in your system's default browser	After executing any `ci - *` or `testInternalDebugCombinedUnitTestCoverage` task, run this task to launch the jacoco reports for viewing.

.idea folder values checked into the git repository

.idea/codeStyles/* - This folder contains all of the shared project codestyle files and is crucial to ensure the whole team is using the same code style.
.idea/runConfigurations/*- This folder contains all of the shared project run configurations. To share a Run Configuration with the team, check the Store as project file checkbox in the Edit Run Configuration window.

Build

Migrated to Versions Catalog: `libs.versions.toml`

buildSrc (legacy approach) is no longer used for dependency/version management.
All dependencies and their versions are now declared in the libs.versions.toml file located in the gradle folder at the root of the project.

Use `ksp` in place of `kapt`

Favor usage of ksp over kapt as it is faster
Supported libs
More links in the ksp declaration in libs.versions.toml

Code

Style

ktlint

Kotlin linter for kotlin code against the Android kotlin style guide to guard against style violations that should be run on the CI and fails builds on a ktlintCheck error.

To prevent ci failures, your development process should include running ktlintFormat prior to pushing any commits to the remote. Example below:

Work on feature/bug making commits until you are ready to push your changes.
Run ktlintFormat and manually fix any issues.
Commit changes (if any).
Push branch (new or updates) to the remote.

Useful gradle tasks that come with the gradle plugin:

// Verification/auto-format
./gradlew ktlintCheck
./gradlew ktlintFormat

// Kotlinscript only verification/format (not very useful)
./gradlew ktlintKotlinScriptCheck
./gradlew ktlintKotlinScriptFormat

// Adds Git pre-commit hook, that runs ktlint check over staged files. (optional)
./gradlew addKtlintCheckGitPreCommitHook

// Adds Git pre-commit hook, that runs ktlint format over staged files and adds fixed files back to commit (optional)
./gradlew addKtlintFormatGitPreCommitHook

More info at https://github.com/JLLeitschuh/ktlint-gradle#configuration

General

Do not use Hungarian notation!. This means no mUser or sUserManager. Instead try user and userManager.

Sealed Classes

Use PascalCase (aka UpperCamelCase) for all objects/classes that are part of (aka extend) a sealed class. Do not use SCREAMING_SNAKE_CASE here!

sealed class Foo {
    object Loading : Foo()
    data class Error(val error: CustomError) : Foo()
    data class Bar(val baz: String) : Foo()
}

References

Kotlin naming rules: http://kotlinlang.org/docs/reference/coding-conventions.html#naming-rules (shows examples for object)
Sealed class API docs: https://kotlinlang.org/docs/reference/sealed-classes.html (shows PascalCase/UpperCamelCase for objects in sealed class)

FIXME/TODO Comments

Use FIXME comments to leave "breadcrumbs" in cases where some specific code or condition needs to be revisited before the next release. You might leave a FIXME when using a placeholder value that needs to be updated with the real one later on down the road when it is created/obtained. During dev signoff, all FIXME entries should be evaluated and handled appropriately.
- // FIXME: Replace with production value from client before release!
- // FIXME: Set to BuildConfig.DEBUG before release!
Use FIXME - VALIDATE EVERY DEV SIGNOFF to validate the condition before every dev signoff. These comments should likely never be removed as long as the commented scenario is still valid to check every dev signoff.
- FIXME - VALIDATE EVERY DEV SIGNOFF: Verify that the play store build is NOT proxyable on 24+ devices.
Use TODO comments to leave "breadcrumbs" of ideas to improve the codebase. You might call out code that you write that could be done in a more optimal way in the future. Or you could leave a TODO for some code that you see that could be improved but you aren't able to immediately act on it right then.
- // TODO: TODO: Consider breaking this up into smaller mappers if the scope becomes too large
- // TODO: Remove this if able after simplifying search results logic

Tip

To display a helpful viewer for TODO and FIXME tags in Android Studio, go to Navigate -> Tool Windows -> TODO

Jetpack Compose

Compose Module

Compose UI code is being developed in a separate module to decrease compile time for Compose previews. This also enforces separation of concerns to avoid references to viewmodels and other non state related code from being coupled to UI code.

Compose Screens

State

Composable screens should be driven by a state object and lambdas. This state class is defined at the screen UI level in the compose module. It can be used to drive previews of the screen easily with fake data, which wouldn't be the case if viewmodels were directly referenced.

data class CreateSnippetScreenState(
    val title: State<String>,
    val filename: State<String>,
    val contents: State<String>,
    val isPrivate: State<Boolean>,
    val creationFailed: State<Boolean>,
    val createEnabled: State<Boolean>,
    val onTitleChanged: (String) -> Unit,
    val onFilenameChanged: (String) -> Unit,
    val onContentsChanged: (String) -> Unit,
    val onPrivateChanged: (Boolean) -> Unit,
    val onCreateClicked: () -> Unit,
)

Connecting ViewModels

View Models hold state in app module and connect this state to the business logic in domain. To connect a view model to a compose state holder we are using a converter function. This function exists in the same module as the viewmodel and does not exist in the compose module.

@Composable
fun CreateSnippetViewModel.toState() = CreateSnippetScreenState(
    title = title.collectAsState(),
    filename = filename.collectAsState(),
    contents = contents.collectAsState(),
    isPrivate = private.collectAsState(),
    creationFailed = failed.collectAsState(),
    createEnabled = createEnabled.collectAsState(),
    onTitleChanged = { title.value = it },
    onFilenameChanged = { filename.value = it },
    onContentsChanged = { contents.value = it },
    onPrivateChanged = { private.value = it },
    onCreateClicked = ::onCreateClick,
)

The Essence of Composition

The real power of composables comes into play as we break down large UI into smaller elements that are easy to reuse. Instead of having a primary button style, we now create a primary button composable function that encapsulates that style into Kotlin code.

@Composable
fun PrimaryButton(
    modifier: Modifier = Modifier,
    buttonText: String,
    forceCaps: Boolean = false,
    onClick: () -> Unit,
    enabled: Boolean = true
) {
    Button(
        onClick = { onClick() },
        modifier = modifier,
        shape = RoundedCornerShape(Dimens.grid_1),
        contentPadding = PaddingValues(Dimens.grid_1_5),
        enabled = enabled,
    ) {
        Text(
            text = if (forceCaps) buttonText.uppercase() else buttonText,
        )
    }
}

Developers should favor breaking large UI elements into composables and passing down state. However, only pass down the state that is required for that specific UI element and avoid coupling to higher level state objects.

Previews

Every composable function that renders UI should have at least one preview.

@Preview(showBackground = true)
@Composable
private fun PreviewPrimaryButton() {
    Preview {
        PrimaryButton(
            buttonText = "Hello World",
            onClick = { },
            modifier = Modifier
                .padding(Dimens.grid_1)
                .fillMaxWidth()
        )
    }
}

Preview All States

If the UI element can have multiple states then it is ideal to exercise each of those states in a preview. For example, if a button can be in a disabled state then create a preview that demonstrates that state.

@Preview(showBackground = true)
@Composable
private fun PreviewPrimaryButtonDisabled() {
    Preview {
        PrimaryButton(
            buttonText = "Hello World",
            onClick = { },
            enabled = false,
            modifier = Modifier
                .padding(Dimens.grid_1)
                .fillMaxWidth()
        )
    }
}

If composables are nested inside other composables then state permutations do not need to be previewed at every level.

Screen Previews

Every screen should be previewed as well. Try to preview as many state combinations as are needed and match Zeplin as close as possible.

@Preview(showSystemUi = true)
@Composable
private fun CreateSnippetScreenPreview() {
    Preview {
        CreateSnippetScreen(
            state = CreateSnippetScreenState(
                title = "".asMutableState(),
                filename = "".asMutableState(),
                contents = "".asMutableState(),
                isPrivate = true.asMutableState(),
                creationFailed = false.asMutableState(),
                createEnabled = true.asMutableState(),
                onCreateClicked = {},
                onTitleChanged = {},
                onFilenameChanged = {},
                onContentsChanged = {},
                onPrivateChanged = {}
            )
        )
    }
}

Previewing Multiple Form Factors

It is important to preview each screen on all possible form factors. To that end, the following annotation has been created in PreviewUtils.kt

@Preview(showSystemUi = true, device = Devices.FOLDABLE)
@Preview(showSystemUi = true, device = Devices.TABLET)
@Preview(showSystemUi = true, device = Devices.DEFAULT)
annotation class PreviewAllDevices

The annotation can be used in the following manner.

@PreviewAllDevices
@Composable
private fun AuthCodePreview() {
    Preview {
        AuthCodeScreen(
            state = AuthCodeState(
                requestUrl = "".asMutableState(),
                devOptionsEnabled = true,
                onAuthCode = {},
                onLoginClicked = {},
                onSignupClicked = {},
                onDevOptionsClicked = {},
                showToolbar = {}
            ),
            navigator = rememberWebViewNavigator()
        )
    }
}

Template Items

There are files in the project marked with either  or // TEMPLATE: .... You can copy whole files or snippets marked with these tags as a basis for a new feature/screen/etc. The point of these existing is to provide a skeleton implementation of a thing(feature/screen/etc) to help speedup development and provide a common/somewhat uniform baseline expectation. To find them easily, you can use Find in Path -> TEMPLATE: or add a filter for TEMPLATE in View -> Tools Windows -> TODO.

KDoc

Add KDoc headers to new interfaces/classes that you create to define what it represents and what it does.
Add KDoc to non-trivial methods explaining expected input and output where appropriate.

Language Injections

Use the @Language("JSON") annotation to add syntax highlighting/editing assistance/error handling to string literals.
Definitely use for JSON and HTML string literals in the app.

No language injection

With language injection you have nice syntax highlighting

... and also language specific error handling

HTML with language injection

Full documentation at https://www.jetbrains.com/help/idea/using-language-injections.html#language_annotation

TimeUnit suffixes

It is a good practice to add the appropriate Time Unit suffix to methods, parameters, properties, and variables on native types such as Long/Int. This makes it explicitly clear what the value actually represents.

Type	GOOD (Clear meaning)	BAD (Ambiguous; have to drill through usages to interpret what value it represents)
variable	`progressMs` OR `progressSeconds`	`progress`
property	`durationMs` OR `durationSeconds`	`duration`
method	`positionMs()` OR `positionSeconds()`	`position()`
parameter	`fun foo(positionMs: Long, durationMs: Long) {}`	`fun foo(position: Long, duration: Long) {}`

If using a type that abstracts the need for a unit such as org.threeten.bp.Duration, then you don't need to specify a suffix on that object. If you pull out the ms value of the duration as a variable, then add the suffix.

Use `StringIdHelper` to avoid depending on `Context`

Use StringIdHelper.* types (representing string resources, format strings, and plurals as well as raw strings) in ViewModels (or other classes) to avoid needing a Context. This allows for simpler unit testing and prevents the need for Context in places it shouldn't be used by pushing the resolution of the final String to the view layer (using the textByStringIdHelper DataBinding custom BindingAdapter function that takes in a Context).

Inject `Clock`

Pass the java.time.Clock instance from Koin to any java 8 date/time APIs that can use it in order to allow control of the clock in unit tests. This will typically be the .now(clock: Clock) API on Instant, ZonedDateTime, LocalDateTime, LocalDate, LocalTime, and so on.

Jacoco

Generate a combined jacoco report of all modules (as well as a report per module) by executing the testDebugCombinedUnitTestCoverage gradle task (or included Android Studio Run Configuration). More info in jacocoRoot.gradle
- View the generated reports by executing the Open Jacoco Report - Debug Android Studio Run configuration, manually executing open_jacoco_report_debug.sh, or manually opening the index.html files from the reports directories.
- You can also just generate the report per module (without the combined report) by executing testDebugUnitTestCoverage (or included Android Studio Run Configuration). More info in jacocoModule.gradle
Fail builds that don't meet the minimum code coverage thresholds per module by executing the testDebugUnitTestCoverageVerification gradle task (or included Android Studio Run Configuration). More info in jacocoModule.gradle

Modularization

Currently the app is broken into two modules: app and data

app module

The top level module.
Uses the com.android.application gradle plugin.

What should be in it

Activity
ViewModels
Android resources such as strings/layouts/nav_graphs/drawables/colors/styles/themes/etc
DI UI Modules
DI Graph creation

data module

The base module.
Uses the com.android.library gradle plugin.
app depends on data.
Make liberal use of the internal modifier where applicable to prevent leaking implementation details from data to modules that consume it (or are dependent on it).

What should be in it

Repositories
Models
Networking
Database
General Utilities including top level functions and objects
DI Data/Networking/Domain Modules
Minimal (or nonexistent) use of context and other Android framework APIs

What should not be in it

Any Android Resources (including usage of Android Resources)

Testing Notes

Test source is isolated per module. For example, app doesn't inherit BaseTest defined in data.
Any testing specific classes (such as BaseTest) needs to be duplicated for each module that needs it.

Build Types/Variants Table

Variant	Application ID	Dev use?	Dev functionality (environment picker, dev options screen, etc)?	Logging (logcat)?	Proxyable? (Charles)	Debuggable?	Signing Keystore?	Proguard/R8?	Built by CI (Jenkins)?	Artifacts Stored (Artifactory/AppCenter)?	QA use?
debug	com.bottlerocketstudios.brarchitecture.debug	Primary local dev machine build	Enabled	Enabled	Yes	Yes	Debug	No	No	No	Rarely/never
productionRelease	com.bottlerocketstudios.brarchitecture	Smoketest before release	Disabled	Disabled	No	No	Release	Yes	Yes	Yes	Primary after Feature Complete and until app release

Generated with https://www.tablesgenerator.com/markdown_tables#

Dev Highlights

Use the Debug variant for day to day development.
Use the productionRelease variant to smoke test that all dev options are disabled (environment picker, etc) and the app is not proxyable. Can also use CI built apk.

Code (Project Specific Items)

Add when things come up.

Gradle Kotlin DSL

General Resources

Tips for migrating from groovy scripts to kotlin gradle script

Full guide: https://guides.gradle.org/migrating-build-logic-from-groovy-to-kotlin/

Takeaways

Change all single quotes to double quotes
Add parentheses in appropriate places for getters and = or () for setters
Ensure groovy scripts still compile
Change filename suffix from .gradle to .gradle.kts and sync -> manually tweak script to eliminate errors

API

Full kotlin gradle dsl api: https://gradle.github.io/kotlin-dsl-docs/api/org.gradle.kotlin.dsl/index.html
General kotlin build script API info at https://gradle.github.io/kotlin-dsl-docs/api/org.gradle.kotlin.dsl/-kotlin-build-script/index.html
General kotlin settings script API info at https://gradle.github.io/kotlin-dsl-docs/api/org.gradle.kotlin.dsl/-kotlin-settings-script/index.html

Default Build Script imports

https://docs.gradle.org/current/userguide/writing_build_scripts.html#script-default-imports

Dealing with task retrieval/creation

Takeaways:

Use delegation APIs available for extras/properties set/get
Don't use map syntax for extra set/get (see below):

There is one last syntax for extra properties that we should cover, one that treats extra as a map. We recommend against using this in general as you lose the benefits of Kotlin’s type checking and it prevents IDEs from providing as much support as they could. However, it is more succinct than the delegated properties syntax and can reasonably be used if you only need to set the value of an extra property without referencing it later.

https://docs.gradle.org/current/userguide/kotlin_dsl.html#kotdsl:containers

Interoperability

Tips/info on dealing with groovy <-> kotlin build script nuances/syntax: https://docs.gradle.org/current/userguide/kotlin_dsl.html#sec:interoperability
Groovy interop sample: https://github.com/gradle/kotlin-dsl/tree/master/samples/groovy-interop

Limitations

https://docs.gradle.org/current/userguide/kotlin_dsl.html#kotdsl:limitations
Don't use org.gradle.configureondemand=true (see below):

We recommend against enabling the incubating configuration on demand feature as it can lead to very hard-to-diagnose problems.

Dependencies.kt

Miscellaneous

Viewing/Editing shared preferences (using AS Device File Explorer)

See https://stackoverflow.com/a/52352741/201939 as well as comments to see how to save as/upload edits on top of the original files.

Inspect/Query/Modify databases (using AS App Inspection / Database Inspector on API 26+ device/emulator)

See https://developer.android.com/studio/inspect/database

Counting lines of code (using cloc)

Install using homebrew: brew install cloc (or your package manager of choice: https://github.com/AlDanial/cloc#install-via-package-manager)
Execute the following snippet via command-line from the root project directory:

# From root project directory:
cloc app/src --by-file-by-lang

FilesExpand file tree

BEST_PRACTICES.md

Latest commit

History

BEST_PRACTICES.md

File metadata and controls

Best Practices

Summary

Target Audience

IDE

Run Configurations Explained

.idea folder values checked into the git repository

Build

Migrated to Versions Catalog: libs.versions.toml

Use ksp in place of kapt

Code

Style

ktlint

General

Sealed Classes

References

FIXME/TODO Comments

Tip

Jetpack Compose

Compose Module

Compose Screens

State

Connecting ViewModels

The Essence of Composition

Previews

Preview All States

Screen Previews

Previewing Multiple Form Factors

Template Items

KDoc

Language Injections

No language injection

With language injection you have nice syntax highlighting

... and also language specific error handling

HTML with language injection

TimeUnit suffixes

Use StringIdHelper to avoid depending on Context

Inject Clock

Jacoco

Modularization

app module

What should be in it

data module

What should be in it

What should not be in it

Testing Notes

Build Types/Variants Table

Dev Highlights

Code (Project Specific Items)

Gradle Kotlin DSL

General Resources

Tips for migrating from groovy scripts to kotlin gradle script

Takeaways

API

Default Build Script imports

Dealing with task retrieval/creation

Interoperability

Limitations

Dependencies.kt

Miscellaneous

Viewing/Editing shared preferences (using AS Device File Explorer)

Inspect/Query/Modify databases (using AS App Inspection / Database Inspector on API 26+ device/emulator)

Counting lines of code (using cloc)

Other Useful General BR Pages

Migrated to Versions Catalog: `libs.versions.toml`

Use `ksp` in place of `kapt`

Use `StringIdHelper` to avoid depending on `Context`

Inject `Clock`