Language Abuse & Unreadability

Readability is one of the most important yet challenging characteristic of a software. Because the code is always readable to its developer, but what about the next developer? As Software Developers, with new technologies, we get excited / hyped quite easily and that doesn't always make the code readable. Especially when the language is as powerful as Kotlin. In this talk, You will see some cases that you can get your code unreadable so fast by abusing the language even though you intend to write readable code.

Presented in
- GDG Porto Android Session Meetup (19.03.2018)
- Kotlin User Group Hamburg Meetup (10.04.2018)

Transcript

1. Language Abuse & Unreadability
3. "Can you read my code?" by Ivan Morgillo Video : https://goo.gl/5fbd1d Presentation: https://goo.gl/ZwRiNF
4. Conventions • Coding Conventions by Kotlin https://kotlinlang.org/docs/reference/coding- conventions.html • Android Kotlin Guides https://android.github.io/kotlin-guides/style.html • Detekt - Static code analysis https://github.com/arturbosch/detekt • An anti-bikeshedding Kotlin linter https://ktlint.github.io/
5. No Package Private private is file visible, internal is module visible
6. Options • Put every class into one file • How readable is it? • How easy to discover them? Extract into module and use • internal instead of private Reconsider the architecture, implementation. •
7. Standard Functions Let - Apply - Also - Run - With
8. /** * Calls the specified function [block] with `this` value as its argument and returns its result. */ @kotlin.internal.InlineOnly public inline fun <T, R> T.let(block: (T) -> R): R = block(this) /** * Calls the specified function [block] and returns its result. */ @kotlin.internal.InlineOnly public inline fun <R> run(block: () -> R): R = block() /** * Calls the specified function [block] with `this` value as its receiver and returns its result. */ @kotlin.internal.InlineOnly public inline fun <T, R> T.run(block: T.() -> R): R = block() /** * Calls the specified function [block] with the given [receiver] as its receiver and returns its result. */ @kotlin.internal.InlineOnly public inline fun <T, R> with(receiver: T, block: T.() -> R): R = receiver.block() /** * Calls the specified function [block] with `this` value as its receiver and returns `this` value. */ @kotlin.internal.InlineOnly public inline fun <T> T.apply(block: T.() -> Unit): T { block(); return this } /** * Calls the specified function [block] with `this` value as its argument and returns `this` value. */ @kotlin.internal.InlineOnly @SinceKotlin("1.1") public inline fun <T> T.also(block: (T) -> Unit): T { block(this); return this }
9. override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View? { return inflater.inflate(R.layout.some_layout, container, false).apply { recyclerView = findViewById(R.id.recycler_view) recyclerView.apply { setHasFixedSize(false) layoutManager = LinearLayoutManager(context) } } } Irrelevant usage
10. override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View? { return inflater.inflate(R.layout.some_layout, container, false).apply { findViewById<RecyclerView>(R.id.recycler_view).apply { setHasFixedSize(false) layoutManager = LinearLayoutManager(context) } } } Irrelevant usage
11. Unnecessary Usage fun getCardHeight(cardId: String): Int { (cardPositionMap[cardId] ?: 0) .let { cardIndex -> val componentListLength = (cardComponentListMap[cardId]?.size ?: 0) return (cardIndex until cardIndex + componentListLength).sumBy { processedHeightList[it] } } }
12. Unnecessary Usage fun getCardHeight(cardId: String): Int { val cardIndex = cardPositionMap[cardId] ?: 0) val componentListLength = (cardComponentListMap[cardId]?.size ?: 0) return (cardIndex until cardIndex + componentListLength).sumBy { processedHeightList[it] } }
13. Unnecessary Usage repoViewModel.getRepositories(pageNumber, shouldFetchFromServer) ... .subscribe( { result -> result?.forEach { item -> run { mAdapter.add(repoViewModel.transformToRepoItem(item)) mAdapter.notifyAdapterDataSetChanged() } } }, { error -> ... }) runOnUiThread
14. Chaining fun RendererAdapter<RendererContent<CardComponent>>.markAsViewed(recyclerView: RecyclerView, id: CardId) { this.collection .indexOfFirst { it.item.storyId == id && it.item.type == CardComponentResponse.Type.COPY_SPREAD } .let { recyclerView.post { notifyItemChanged(it, CopySpreadEvent.CardViewed()) } if (it > 0) { collection[it] } else { null } } ?.item ?.hasBeenViewed = true }
15. Chaining fun RendererAdapter<RendererContent<CardComponent>>.markAsViewed(recyclerView: RecyclerView, id: CardId) { val firstIndex = this.collection.indexOfFirst { it.item.storyId == id && it.item.type == CardComponentResponse.Type.COPY_SPREAD } recyclerView.post { notifyItemChanged(firstIndex, CopySpreadEvent.CardViewed()) } if (firstIndex > 0) collection[firstIndex].item.hasBeenViewed = true }
16. Chaining
17. When
18. Not an If when (enabled) { true -> view.enableSend() false -> view.disableSend() } if (enabled) { view.enableSend() } else { view.disableSend() }
19. Compile time check https://goo.gl/NSXJEA fun access(employee: Employee, contract: Contract): SafariBookAccess { return when (employee) { SENIOR_ENGINEER -> when (contract) { PROBATION -> NotGranted(AssertionError("Not allowed for probation.")) PERMANENT -> Granted(DateTime()) CONTRACTOR -> Granted(DateTime()) } REGULAR_ENGINEER -> when (contract) { PROBATION -> NotGranted(AssertionError("Not allowed for probation.")) PERMANENT -> Granted(DateTime()) CONTRACTOR -> Blocked("Access blocked for $contract.") } JUNIOR_ENGINEER -> when (contract) { PROBATION -> NotGranted(AssertionError("Not allowed for probation.")) PERMANENT -> Blocked("Access blocked for $contract.") CONTRACTOR -> Blocked("Access blocked for $contract.") } else -> throw AssertionError() } }
20. Compile time check https://goo.gl/NSXJEA fun access(contract: Contract, employee: Employee) = when (Pair(contract, employee)) { Pair(PROBATION, SENIOR_ENGINEER), Pair(PROBATION, REGULAR_ENGINEER), Pair(PROBATION, JUNIOR_ENGINEER) -> NotGranted(AssertionError("Not allowed for probation.”)) Pair(PERMANENT, SENIOR_ENGINEER), Pair(PERMANENT, REGULAR_ENGINEER), Pair(PERMANENT, JUNIOR_ENGINEER), Pair(CONTRACTOR, SENIOR_ENGINEER) -> Granted(DateTime(1)) Pair(CONTRACTOR, REGULAR_ENGINEER), Pair(CONTRACTOR, JUNIOR_ENGINEER) -> Blocked("Access for junior contractors is blocked.") else -> throw AssertionError("Unsupported case of $employee and $contract") }
21. Compile time check https://goo.gl/NSXJEA fun access(contract: Contract, employee: Employee) = when (Pair(contract, employee)) { Pair(PROBATION, SENIOR_ENGINEER), Pair(PROBATION, REGULAR_ENGINEER), Pair(PROBATION, JUNIOR_ENGINEER) -> NotGranted(AssertionError("Not allowed for probation.”)) Pair(PERMANENT, SENIOR_ENGINEER), Pair(PERMANENT, JUNIOR_ENGINEER), Pair(CONTRACTOR, SENIOR_ENGINEER) -> Granted(DateTime(1)) Pair(CONTRACTOR, REGULAR_ENGINEER), Pair(CONTRACTOR, JUNIOR_ENGINEER) -> Blocked("Access for junior contractors is blocked.") else -> throw AssertionError("Unsupported case of $employee and $contract") }
22. Compile time check https://goo.gl/NSXJEA fun access(contract: Contract, employee: Employee) = when (Pair(contract, employee)) { Pair(PROBATION, SENIOR_ENGINEER), Pair(PROBATION, REGULAR_ENGINEER), Pair(PERMANENT, JUNIOR_ENGINEER), Pair(PROBATION, JUNIOR_ENGINEER) -> NotGranted(AssertionError("Not allowed for probation.”)) Pair(PERMANENT, SENIOR_ENGINEER), Pair(PERMANENT, JUNIOR_ENGINEER), Pair(CONTRACTOR, SENIOR_ENGINEER) -> Granted(DateTime(1)) Pair(CONTRACTOR, REGULAR_ENGINEER), Pair(CONTRACTOR, JUNIOR_ENGINEER) -> Blocked("Access for junior contractors is blocked.") else -> throw AssertionError("Unsupported case of $employee and $contract") }
23. Extensions Let’s not fall into Extensions Hell while complaining about Utils Hell :)
24. Unique Naming
25. Unique Naming Conflicting overloads: public fun String.check(): Boolean defined in com.xing.sample.talk in file VerificationExtension.kt, public fun String.check(): Boolean defined in com.xing.sample.talk in file NamingExtension.kt
26. Top level const const val FIELD = "field"
27. Make them • private or internal Use companion object • Keep only global ones as top level • Constants
28. No Java in Kotlin Especially with null checks
29. fun transformToRepoItem(repo: Repo): RepoItem { val item = RepoItem() if (repo.name != null) item.name = repo.name!! if (repo.description != null) item.description = repo.description!! if (repo.owner != null && repo.owner!!.login != null) item.login = repo.owner!!.login!! if (repo.fork != null) item.fork = repo.fork!! if (repo.htmlUrl != null) item.repoURL = repo.htmlUrl!! if (repo.owner != null && repo.owner!!.htmlUrl != null) item.ownerURL = repo.owner!!.htmlUrl!! return item }
30. fun transformToRepoItem(repo: Repo): RepoItem { return RepoItem().apply { name = repo.name ?: "" description = repo.description ?: "" login = repo.owner?.login ?: "" fork = repo.fork ?: false repoURL = repo.htmlUrl ?: "" ownerURL = repo.owner?.htmlUrl ?: "" } }
31. Find the Kotlin way! But still keep it readable, for next person. Thanks