You can call it beautiful code when the code also makes it look like the language was made for the problem
Ward Cunningham
Functions
Photo by Alejandro Lopez on Unsplash
✅ Function names should be descriptive and consistent
✅ Output args should be avoided in place of return values
Clean Code - the good
✅ Functions should be commands or queries but not both
Duplication may be the root of all evil in software
🤔
Clean Code - the questionable
🤔 Don't repeat yourself - DRY
Programmer A sees duplication
Programmer A extracts duplication and gives it a name
Programmer A replaces the duplication with the new abstraction
Time passes
A new requirements appears for which the current abstraction is almost perfect
Programmer B gets tasked to implement this requirement
Programmer B feels honor-bound to retain the existing abstraction, but since it isn't exactly the same they have to alter the code to take a parameter.
Another new requirement arrives. Programmer X - another param, another conditional.
Repeat until code becomes unmaintainable
@Composable
fun OutlinedButton(
onClick: () -> Unit,
modifier: Modifier = Modifier,
enabled: Boolean = true,
shape: Shape = ButtonDefaults.outlinedShape,
colors: ButtonColors = ButtonDefaults.outlinedButtonColors(),
elevation: ButtonElevation? = null,
border: BorderStroke? = ButtonDefaults.outlinedButtonBorder,
contentPadding: PaddingValues = ButtonDefaults.ContentPadding,
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
content: @Composable RowScope.() -> Unit
) =
Button(
onClick = onClick,
modifier = modifier,
enabled = enabled,
shape = shape,
colors = colors,
elevation = elevation,
border = border,
contentPadding = contentPadding,
interactionSource = interactionSource,
content = content
)
OutlinedButton.kt
@Composable
fun TextButton(
onClick: () -> Unit,
modifier: Modifier = Modifier,
enabled: Boolean = true,
shape: Shape = TextButtonDefaults.shape,
colors: TextButtonColors = TextButtonDefaults.textButtonColors(),
border: BorderStroke? = null,
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
content: @Composable BoxScope.() -> Unit,
) {
androidx.wear.compose.materialcore.Button(
onClick = onClick,
modifier.minimumInteractiveComponentSize(),
enabled = enabled,
backgroundColor = { colors.containerColor(enabled = it) },
interactionSource = interactionSource,
shape = shape,
border = { rememberUpdatedState(border) },
buttonSize = TextButtonDefaults.DefaultButtonSize,
content = provideScopeContent(
colors.contentColor(enabled = enabled),
MaterialTheme.typography.buttonMedium,
content
)
)
}
TextButton.kt
abstract class BaseViewModel<T>: ViewModel() {
abstract val viewState: BehaviorSubject<T>
}abstract class BaseViewModel<T>: ViewModel() {
abstract val viewState: BehaviorSubject<T>
}
abstract class BaseViewModel<T>: ViewModel() {
abstract val viewState: StateFlow<T>
}😨
class HomeViewModel: ViewModel() {
val viewState: BehaviorSubject<Home.ViewState>
}class SettingsViewModel: ViewModel() {
val viewState: BehaviorSubject<Settings.ViewState>
}In large Android apps, reducing coupling trumps DRY
Clean Code - the weird 😦
An ideal function has zero arguments
The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible.
😦
@Composable
fun Button(
onClick: () -> Unit,
modifier: Modifier = Modifier,
enabled: Boolean = true,
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
elevation: ButtonElevation? = ButtonDefaults.elevation(),
shape: Shape = MaterialTheme.shapes.small,
border: BorderStroke? = null,
colors: ButtonColors = ButtonDefaults.buttonColors(),
contentPadding: PaddingValues = ButtonDefaults.ContentPadding,
content: @Composable RowScope.() -> Unit
)@Composable
fun Button(
params: ButtonParams,
)
class ButtonParams(
val onClick: () -> Unit,
val modifier: Modifier = Modifier,
val enabled: Boolean = true,
val interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
val elevation: ButtonElevation? = ButtonDefaults.elevation(),
val shape: Shape = MaterialTheme.shapes.small,
val border: BorderStroke? = null,
val colors: ButtonColors = ButtonDefaults.buttonColors(),
val contentPadding: PaddingValues = ButtonDefaults.ContentPadding,
val content: @Composable RowScope.() -> Unit
)🤢
Side effects are lies. Your function promises to do one thing, but it also does other hidden things.
Zero args, but no side effects?
Sometimes it will make unexpected changes to the variables of its own class
😦
The first rule of functions is that they should be small
...every function in [Kent Beck's] program was just two, or three, or four lines long...
..that's how short your functions should be
😦
@Throws(Exception::class)
fun testableHtml(pageData: PageData, includeSuiteSetup: Boolean): String? {
val wikiPage: WikiPage = pageData.getWikiPage()
val buffer = StringBuffer()
if (pageData.hasAttribute("Test")) {
if (includeSuiteSetup) {
val suiteSetup: WikiPage? = PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, wikiPage
)
if (suiteSetup != null) {
val pagePath: WikiPagePath = suiteSetup.getPageCrawler().getFullPath(suiteSetup)
val pagePathName: String = PathParser.render(pagePath)
buffer.append("!include -setup .").append(pagePathName).append("\n")
}
}
val setup: WikiPage? = PageCrawlerImpl.getInheritedPage("SetUp", wikiPage)
if (setup != null) {
val setupPath: WikiPagePath = wikiPage.getPageCrawler().getFullPath(setup)
val setupPathName: String = PathParser.render(setupPath)
buffer.append("!include -setup .").append(setupPathName).append("\n")
}
}
buffer.append(pageData.getContent())
if (pageData.hasAttribute("Test")) {
val teardown: WikiPage? = PageCrawlerImpl.getInheritedPage("TearDown", wikiPage)
if (teardown != null) {
val tearDownPath: WikiPagePath = wikiPage.getPageCrawler().getFullPath(teardown)
val tearDownPathName: String = PathParser.render(tearDownPath)
buffer.append("\n").append("!include -teardown .").append(tearDownPathName).append("\n")
}
// snip
}
pageData.setContent(buffer.toString())
return pageData.getHtml()
}@Throws(Exception::class)
fun testableHtml(pageData: PageData, includeSuiteSetup: Boolean): String? {
val wikiPage: WikiPage = pageData.getWikiPage()
val buffer = StringBuffer()
if (pageData.hasAttribute("Test")) {
if (includeSuiteSetup) {
val suiteSetup: WikiPage? = PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, wikiPage
)
if (suiteSetup != null) {
val pagePath: WikiPagePath = suiteSetup.getPageCrawler().getFullPath(suiteSetup)
val pagePathName: String = PathParser.render(pagePath)
buffer.append("!include -setup .").append(pagePathName).append("\n")
}
}
val setup: WikiPage? = PageCrawlerImpl.getInheritedPage("SetUp", wikiPage)
if (setup != null) {
val setupPath: WikiPagePath = wikiPage.getPageCrawler().getFullPath(setup)
val setupPathName: String = PathParser.render(setupPath)
buffer.append("!include -setup .").append(setupPathName).append("\n")
}
}
buffer.append(pageData.getContent())
if (pageData.hasAttribute("Test")) {
val teardown: WikiPage? = PageCrawlerImpl.getInheritedPage("TearDown", wikiPage)
if (teardown != null) {
val tearDownPath: WikiPagePath = wikiPage.getPageCrawler().getFullPath(teardown)
val tearDownPathName: String = PathParser.render(tearDownPath)
buffer.append("\n").append("!include -teardown .").append(tearDownPathName).append("\n")
}
// snip
}
pageData.setContent(buffer.toString())
return pageData.getHtml()
}class SetupTeardownIncluder private constructor(private val pageData: PageData) {
private lateinit var isSuite: Boolean
@Throws(Exception::class)
fun render(isSuite: Boolean): String? {
this.isSuite = isSuite
if (isTestPage()) includeSetupAndTeardownPages()
return pageData.getHtml()
}
@Throws(Exception::class)
private fun includeSetupAndTeardownPages() {
includeSetupPages()
includePageContent()
includeTeardownPages()
updatePageContent()
}
@Throws(Exception::class)
private fun includeSetupPages() {
if (isSuite) includeSuiteSetupPage()
includeSetupPage()
}
@Throws(Exception::class)
private fun includeSuiteSetupPage() {
include(SuiteResponder.SUITE_SETUP_NAME, "-setup")
}
@Throws(Exception::class)
private fun includeSetupPage() {
include("SetUp", "-setup")
}
@Throws(Exception::class)
private fun updatePageContent() {
pageData.setContent(newPageContent.toString())
}
}class SetupTeardownIncluder private constructor(
private val pageData: PageData
) {
@Throws(Exception::class)
fun render(isSuite: Boolean): String? {
this.isSuite = isSuite
if (isTestPage()) includeSetupAndTeardownPages()
return pageData.getHtml()
}
@Throws(Exception::class)
private fun includeSetupAndTeardownPages() {
includeSetupPages()
includePageContent()
includeTeardownPages()
updatePageContent()
}
@Throws(Exception::class)
private fun includeSetupPages() {
if (isSuite) includeSuiteSetupPage()
includeSetupPage()
}
@Throws(Exception::class)
private fun includeSuiteSetupPage() {
include(SuiteResponder.SUITE_SETUP_NAME, "-setup")
}
@Throws(Exception::class)
private fun includeSetupPage() {
include("SetUp", "-setup")
}
@Throws(Exception::class)
private fun updatePageContent() {
pageData.setContent(newPageContent.toString())
}
}Implied:
A clean functional decomposition uses very short niladic functions and side effects on members to summarize a problem in a way that approaches natural language
IEEE Technical Committee on Distributed Processing Outstanding Technical Contribution Award (2020)
Elected to National Academy of Engineering (2001)
ACM Grace Murray Hopper Award (1987)
John Ousterhout
Jeff Dean
Sanjay Ghemawat
Copyright Peter Adams Photography
[CS190 at Stanford] is taught in a fashion similar to a traditional English writing class. In an English class, students use an iterative process where they write a draft, get feedback, and then rewrite to make improvements.
In CS 190, students develop a substantial piece of software from scratch. We then go through extensive code reviews to identify design problems, and students revise their projects to fix the problems.
Photo by Aaron Burden
https://unsplash.com/@aaronburden
Modules should be deep
Photo by Jonas Allert on Unsplash
Module
A software system is decomposed into a collection of modules that are relatively independent
Interface (cost: less is better)
Functionality (benefit: more is better)
Deep Module
Shallow Module
Another example of a deep module is the garbage collector in a language such as Go or Java. This module has no interface at all; it works invisibly behind the scenes to reclaim unused memory.
Photo by James Day www.jamesdayart.com
val retrofit = Retrofit.Builder()
.baseUrl("https://api.github.com")
.build()
interface GitHub {
@GET("/repos/{owner}/{repo}/contributors")
suspend fun contributors(
@Path("owner") owner: String?,
@Path("repo") repo: String?
): Response<String>
}
val gitHubService = retrofit.create(GitHub::class.java)
A deep module in Android
Shallow method
private fun addNullValueForAttribute(attribute: String) {
data[attribute] = null
}No abstraction - all of the functionality is visible through its interface
No simpler to think about the interface than to think about the full implementation
More keystrokes to invoke the function than it would take for a caller to manipulate the data property directly
Adds complexity (in the form of a new interface for developers to learn) but provides no compensating benefit
class SetupTeardownIncluder private constructor(private val pageData: PageData) {
@Throws(Exception::class)
fun includeSetupAndTeardownPages() {
includeSetupPages() // these are not deep!
includePageContent()
includeTeardownPages()
updatePageContent()
}
@Throws(Exception::class)
private fun includeSetupPages() {
if (isSuite) includeSuiteSetupPage()
includeSetupPage()
}
@Throws(Exception::class)
private fun includeSuiteSetupPage() {
include(SuiteResponder.SUITE_SETUP_NAME, "-setup")
}
@Throws(Exception::class)
private fun includeSetupPage() {
include("SetUp", "-setup")
}
@Throws(Exception::class)
private fun updatePageContent() {
pageData.setContent(newPageContent.toString())
}
} @Throws(Exception::class)
fun includeSetupAndTeardownPages(isSuite: Boolean) {
if (isSuite) include(SuiteResponder.SUITE_SETUP_NAME, "-setup")
include("SetUp", "-setup")
newPageContent.append(pageData.getContent())
include("TearDown", "-teardown")
if (isSuite) include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown")
pageData.setContent(newPageContent.toString())
} @Throws(Exception::class)
private fun include(pageName: String, arg: String) {
val inheritedPage = findInheritedPage(pageName)
if (inheritedPage != null) {
val pagePathName = getPathNameForPage(inheritedPage)
newPageContent
.append("\n!include ")
.append(arg)
.append(" .")
.append(pagePathName)
.append("\n")
}
}this is deep, let's keep it!
class MyFragment : Fragment() {
private val viewModel: MyViewModel by viewModel()
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
collectFlows()
}
private fun collectFlows() {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
launch {
collectViewStates()
}
launch {
collectEvents()
}
}
}
}
private suspend fun collectViewStates() {
viewModel.viewStates.collect {
binding.update(it)
}
}
private suspend fun collectEvents() {
viewModel.viewStates.collect {
handle(it)
}
}
}
class MyFragment : Fragment() {
private val viewModel: MyViewModel by viewModel()
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
launch {
viewModel.viewStates.collect {
binding.update(it)
}
}
launch {
viewModel.events.collect {
handle(it)
}
}
}
}
}
}
Abstract
From Middle English abstract, borrowed from Latin abstractus, perfect passive participle of abstrahō (“draw away”), formed from abs- (“away”) + trahō (“to pull, draw”).
Abstraction
An abstraction is a simplified view of an entity, which omits unimportant details
An abstraction can go wrong in two ways:
- By including details that are not really important
- By omitting details that really are important
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
collectFlows()
}Which flows?
When are they collected?
Omits important details
To sum up: If a function is only called from a single place, consider inlining it.
Besides awareness of the actual code being executed, inlining functions also has the benefit of not making it possible to call the function from other places.
deep == magic?
Magic Frameworks are bad..."Magic" is when a framework does something automatically, in a cool way.
🤔
Photo by Joe Zlomek on Unsplash
http://www.softwareonthebrain.com/2022/01/the-misunderstood-single-responsibility.html
Classes
Photo by Nathan Dumlao on Unsplash
SOLID
Single responsibility
Open/closed
Liskov substitution
Interface segregation
Dependency inversion
Single responsibility
There should never be more than one reason for a class to change. In other words, every class should have only one responsibility.
The Single Responsibility Principle says that code should only do one thing. Any non-trivial code can have any number of reasons to change, which may or may not include the one you had in mind.
interface Call {
fun request(): Request
fun enqueue(responseCallback: Callback)
fun cancel()
fun isExecuted(): Boolean
fun isCanceled(): Boolean
fun clone(): Call
}interface Call {
fun request(): Request
fun enqueue(responseCallback: Callback)
fun clone(): Call
fun executionManager(): ExecutionManager
}
interface ExecutionManager { // WTF?
fun cancel()
fun isExecuted(): Boolean
fun isCanceled(): Boolean
}Should we split this class to make for a "single reason to change"?
The Misunderstood Single Responsibility Principle
I recently read Adaptive Code which, as of today, has a 4.7/5 rating on Amazon with 123 ratings. There is a whole chapter on the SRP. And throughout, it incorrectly explains the SRP as relating to classes doing too many things and justifies design decisions based on that alone.
Often people who like tiny classes will use it to justify making many more even tinier classes.
What starts as a simple, easy-to-understand class ends up being broken into many abstractions that are now harder to understand collectively.
When deciding whether to combine or separate, the goal is to reduce the complexity of the system as a whole and improve its modularity.
It might appear that the best way to achieve the goal is to divide the system into a large number of small components: the smaller the components, the simpler each individual component is likely to be.
- Complexity comes just from the [sheer] number of components
- Subdivision can result in additional code to manage the components
- Subdivision can result in duplication
- Subdivision creates separation
Costs of subdivision
for example, methods that were together in a single class before subdivision may be in different classes after subdivision, and possibly in different files.
if there are dependencies between the components, then separation is bad: developers will end up flipping back and forth between the components.
Photo by Pablo Varela on Unsplash
Deflating "single responsibility"
The principle actually means that the code in a given module has to be owned by one and only one ultimate business owner (or functional unit). If that isn't true you have to break it up.
Joe Lynch
CFO and COO both depend on the code that calculates employee hours. Calculating hours is simple math that everyone should agree on, right?
⏰
One executive requests a change to that code and it breaks the other's business rule. The hours calculation cannot be shared: that piece of code should have a Finance version and an Operations version, despite the fact that that violates our DRY and cohesion sensibilities.
Photo by Matt Walsh on Unsplash
1. Bring together if information is shared
chat_layout.xml
ChatViewModel.kt
Slides inspired by Leland Richardson
1. Bring together if information is shared
ChatScreen.kt
ChatViewModel.kt
Slides inspired by Leland Richardson
sealed interface LoginUiState {
object Loading: LoginUiState
data class UserLoggedIn(val user: User): LoginUiState
data class ShowToast(val message: String): LoginUiState
}
class LoginViewModel: ViewModel() {
// ...
fun login() {
try {
// ...
} catch (e: Exception) {
emitState(LoginUiState.ShowToast("Failed to login"))
}
}
fun resetPassword() {
try {
// ...
} catch (e: Exception) {
emitState(LoginUiState.ShowToast("Operation failed!"))
}
}
}
ViewModel now resembles View, just like Leland warned us 😢
sealed interface LoginUiState {
object Loading: LoginUiState
data class UserLoggedIn(val user: User): LoginUiState
data class ShowToast(val message: String): LoginUiState
}
sealed interface LoginUiState {
object Loading: LoginUiState
data class LoginFailed(val reason: String): LoginUiState
data class ResetFailed(val reason: String): LoginUiState
}
2. Bring together if it will simplify the interface
3. Bring together to eliminate duplication
4. Separate general-purpose and special-purpose code
If a module contains a mechanism that can be used for several different purposes, then it should provide just that one general-purpose mechanism.
It should not include code that specializes the mechanism for a particular use, nor should it contain other general purpose-mechanisms.
class Node {
val children = mutableListOf<Node>()
}
class NodeApplier(node: Node) : AbstractApplier<Node>(node) {
override fun onClear() {}
override fun insertBottomUp(index: Int, instance: Node) {}
override fun insertTopDown(index: Int, instance: Node) {
current.children.add(index, instance)
}
override fun move(from: Int, to: Int, count: Int) {
current.children.move(from, to, count)
}
override fun remove(index: Int, count: Int) {
current.children.remove(index, count)
}
}
Hypothetical Compose monolith
node management
Material UI
Mixing node management with UI precludes other uses like Molecule
Not seeing Material UI as a special case of UI precludes other UI
What about minimizing coupling and maximizing cohesion? 🤔
Photo by Jukan Tateisi on Unsplash
@d_rawers
@drawers@androiddev.social
A Philosophy of Android Software Design - GDG DevFest Auckland
By David Rawson
