Share
Code readability: Session 5 (ver. 2, En)
Session 5: Function
List of sessions:
Session 1: Introduction and Principles
- https://speakerdeck.com/munetoshi/code-readability-session-1-ver-2-en
Session 2: Naming
- https://speakerdeck.com/munetoshi/code-readability-session-2-ver-2-en
Session 3: Comments
- https://speakerdeck.com/munetoshi/code-readability-session-3-ver-2-en
Session 4: State
- https://speakerdeck.com/munetoshi/code-readability-session-4-ver-2-en
Session 5: Function
- https://speakerdeck.com/munetoshi/code-readability-session-5-ver-2-en
Session 6: Dependency I
- https://speakerdeck.com/munetoshi/code-readability-session-6-ver-2-en
Session 7: Dependency II
- https://speakerdeck.com/munetoshi/code-readability-session-7-ver-2-en
Session 8: Review
- https://speakerdeck.com/munetoshi/code-readability-session-8-ver-2-en
Previous version: https://speakerdeck.com/munetoshi/code-readability
For the Japanese version, refer to https://gist.github.com/munetoshi/65a1b563fb2c271f328c121a4ac63571
© 2019-2023 Munetoshi Ishikawa, supported by LINE corporation
Munetoshi Ishikawa
Transcript
60 slides
-
Brushup: State Simplify state for readability and robustness - Do
not make it an objective Avoid non-orthogonal relationships - Replace with function or use sum-type Care about state transition - Immutability, idempotence, and cycle -
Contents of this lecture - Introduction and Principles - Natural
language: Naming, Comments - Inner type structure: State, Function - Inter type structure: Dependency I, Dependency II - Follow-up: Review Function > Introduction -
"Function" in this session Also includes - Subroutine - Procedure
- Method - Computed property - Constructor, initialization block - ... Function > Introduction -
What a readable function is Predictable - Consistent with the
name - Easy to write documentation - Few error cases or limitations Function > Introduction -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Introduction -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Responsibility > Check with a short summary -
Split the function if it is hard to summarize Try
to summarize what the function does Function > Responsibility > Check with a short summary -
Function summary: Example 1/2 Question: How can we summarize this
code? messageView.text = messageData.contentText senderNameView.text = messageData.senderName timestampView.text = messageData.sentTimeText Example answer: "Bind/update message layout with a new message data" Function > Responsibility > Check with a short summary -
Function summary: Example 2/2 Question: How can we summarize this
code? messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } Example answer?: "Bind a new message and save it" "Perform actions on a new message received" Function > Responsibility > Check with a short summary -
Function responsibility and summary Hard to summarize = May need
to split the function fun bindMessageViewData(messageData: MessageData) { ... } fun saveMessageDataToDatabase(messageData: MessageData) { ... } Exceptions: - Interface for other layers/modules - Entry point of event handling Function > Responsibility > Check with a short summary -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Responsibility > Command and query -
Command-query separation6 Separate a function by command and query -
Command: Modifies the receiver or parameters - Query: Return without any modification 6 Bertrand Meyer. 1988. Object-oriented Software Construction. Prentice-Hall Function > Responsibility > Command and query -
Command-query separation: Example 1/2 class IntList(vararg elements: Int) { infix
fun append(others: IntList): IntList = ... } val a = IntList(1, 2) val b = IntList(3, 4) val c = a append b Question: What is the expected value of a, b and c? Function > Responsibility > Command and query -
Command-query separation: Example 2/2 val a = IntList(1, 2) val
b = IntList(3, 4) val c = a append b Expected: a={1, 2}, b={3, 4}, c={1, 2, 3, 4} Surprising result: a={1, 2, 3, 4}, b={3, 4}, c={1, 2, 3, 4} Function append should not modify a or b because it returns a result Function > Responsibility > Command and query -
Command-query separation: Drawbacks 1/3 Command-query separation is not an objective
- May cause strong coupling (will explain during the next session) - May make an unnecessary state Function > Responsibility > Command and query -
Command-query separation: Drawbacks 2/3 Bad code example class UserDataStore {
private var latestOperationResult: Result = NO_OPERATION // Command fun saveUserData(userData: UserData) = ... // Query val wasLatestOperationSuccessful: Boolean get() = ... } Variable latestOperationResult may become a cause of a bug Function > Responsibility > Command and query -
Command-query separation: Drawbacks 3/3 Good code example class UserDataRequester {
/** * Saves a given user data to ... * Then, returns true if the operation successful, otherwise false. */ fun saveUserData(userData: UserData): Boolean = ... } No need to keep the latest result when returning it directly Function > Responsibility > Command and query -
Responsibility on return value and modification Do not modify when
returning a "main" result = Acceptable to return a "sub" result with modification - Main result: conversion/calculation result, new instance, ... - Sub result: error value, metadata of modification (stored size), ... (Documentation is mandatory) Acceptable if the interface is a de facto standard e.g., Queue<E>.poll(): E, Iterator<E>.next(): E Function > Responsibility > Command and query -
Make responsibility clear: Summary Try to split a function if
it is hard to summarize - Try to write documentation anyway Do not modify when returning a "main" result - Fine to return a "sub" result with modification - Fine if the interface is a de facto standard Function > Responsibility > Summary -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Flow -
Function with clear flow Important point: Can understand the code
overview only by scanning through - Able to skip reading details - Easy to find the important part - No need to cover all conditional branches Confirm with writing the short summary Function > Flow -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Flow > Definition-based programming -
Definition-based programming 1/2 Style that frequently uses named variables and
functions Objective: - Clarify code that can be skipped - Make abstraction level high - Reduce the need to go back and read Function > Flow > Definition-based programming -
Definition-based programming 2/2 Replace hard-to-read elements with named variables and
functions Hard-to-read elements: - Nest (parameter, function, class, control flow) - Anonymous function, object literal - Method chain Function > Flow > Definition-based programming -
Parameter nest: Issues 1/2 Question: Why is the following code
difficult to read? showDialogOnError( presenter.updateSelfProfileView( repository.queryUserModel(userId) ) ) Function > Flow > Definition-based programming -
Parameter nest: Issues 2/2 Answer: - The important function call
is placed inside the nest - It requires to understand all return values - The direction of code flow is reversed (bottom to top) Function > Flow > Definition-based programming -
Parameter nest: How to fix Use a named local variable
for return values val userModel = repository.queryUserModel(userId) val viewUpdateResult = presenter.updateSelfProfileView(UserModel) showDialogOnError(viewUpdateResult) Function > Flow > Definition-based programming -
Method chain with lambda: Issues 1/2 Question: Why is the
following code difficult to read? return queryTeamMemberIds(teamId) .map { memberId -> ... // Convert to UserModel ... } .filter { val userStatus = ... userStatus.isOnline } .map { it.emailAddress } Function > Flow > Definition-based programming -
Method chain with lambda: Issues 2/2 Answer: Need to read
details to understand behavior - Receiver for each method call (= return value for each call) - Behavior detail of each lambda Function > Flow > Definition-based programming -
Method chain with lambda: How to fix 1/2 Option 1:
Split the chain with named local variables val teamMembers = queryTeamMemberIds(teamId).map { ... ... } val onlineTeamMembers = teamMembers.filter { ... ... } return onlineTeamMembers.map { it.emailAddress } It may still be difficult to read if there are long lambdas Function > Flow > Definition-based programming -
Method chain with lambda: How to fix 2/2 Option 2:
Replace lambdas with named private functions return queryTeamMemberIds(teamId) .map(::toUserModel) .filter(::isOnline) .map { it.emailAddress } ... private fun toUserModel(userId: UserId): UserModel { ... } private fun isOnline(userModel: UserModel): Boolean { ... } Function > Flow > Definition-based programming -
Definition-based programming: Pitfall Extracting may make the code less readable
- Unnecessary state - Unnecessary strong coupling (will explain at the next session) Function > Flow > Definition-based programming -
Pitfalls of extraction: Original code val userNameTextView: View = ...
val profileImageView: View = ... init { // Complex userNameTextView initialization code userNameTextView... // Complex profileImageView initialization code profileImageView... } Extract complex initialization code Function > Flow > Definition-based programming -
Pitfalls of extraction: Bad example var userNameTextView: View? = null
var profileImageView: View? = null init { initializeUserNameTextView() initializeProfileImageView() } private fun initializeUserNameTextView() { userNameTextView = ... private fun initializeProfileImageView() { profileImageView = ... Function > Flow > Definition-based programming -
Pitfalls of extraction: What is wrong - Unnecessary mutability and
nullability - Unsafe to call initialize... methods multiple times - Function name initialize... is ambiguous Optimize the scope of the extracted methods Function > Flow > Definition-based programming -
Pitfalls of extraction: Good example Extract view creation and initialization
logic val userNameTextView: View = createUserNameTextView() val profileImageView: View = createProfileImageView() private fun createUserNameTextView(): TextView = ... private fun createProfileImageView(): ImageView = ... Function > Flow > Definition-based programming -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Flow > Focus on a happy path -
Happy path and unhappy path Happy path: Achieves the main
objective of a function Unhappy path: Does not achieve the main objective of a function Function > Flow > Focus on a happy path -
Happy path and unhappy path: Example String.toIntOrNull() Happy path: Returns
an integer value e.g, "-1234", "0", "+001", "-2147483648" Unhappy path: Returns null e.g., "--0", "text", "", "2147483648", "1.0" Function > Flow > Focus on a happy path -
Function flow and happy path Firstly, filter unhappy paths to
focus on a happy path Apply early return Function > Flow > Focus on a happy path -
Early return: Bad example if (isNetworkAvailable()) { val queryResult =
queryToServer() if (queryResult.isValid) { // Do something with queryResult... } else { showInvalidResponseDialog() } } else { showNetworkUnavailableDialog() } Function > Flow > Focus on a happy path -
Early return: What is wrong - Hard to find the
main case logic - The relation between error condition and handling logic Function > Flow > Focus on a happy path -
Early return: Good example if (!isNetworkAvailable()) { showNetworkUnavailableDialog() return }
val queryResult = queryToServer() if (!queryResult.isValid) { showInvalidResponseDialog() return } // Do something with queryResult... Function > Flow > Focus on a happy path -
Early return: Caution 1/2 Avoid early return with only some
branches of when/switch May overlook the early return and the corresponding condition val messageText = when (type) { FOO -> "this is FOO" BAR -> "this is BAR" null -> return } Function > Flow > Focus on a happy path -
Early return: Caution 1/2 Avoid early return with only some
branches of when/switch May overlook the early return and the corresponding condition if (type == null) { return } val messageText = when (type) { FOO -> "this is FOO" BAR -> "this is BAR" } Function > Flow > Focus on a happy path -
Early return: Caution 2/2 Avoid making an unnecessary unhappy path
Typical case: Iteration of an empty collection if (list.isEmpty) { return listOf() } return list .filter { ... } .map { ... } Function > Flow > Focus on a happy path -
Topics - Make responsibility clear - Split if it is
hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on a happy path - Split by object, not condition Function > Flow > Split by object -
Axis to break down function 1/2 Multiple conditions and target
objects Example: Show account state - Conditions: Two account types (premium, free) - Objects: Two views (background, icon) Function > Flow > Split by object -
Axis to break down function 2/2 Question: On which axis
should the function be split? Condition Object Premium account Free account Background color Account icon Red Gray “Twemoji” ©Twitter, Inc and other contributors (Licensed under CC-BY 4.0) https://twemoji.twitter.com/ Function > Flow > Split by object -
Bad example of "split by condition first" fun bindViewData(accountType: AccountType)
{ when (accountType) { PREMIUM -> updateViewsForPremium() FREE -> updateViewsForFree() } } private fun updateViewsForPremium() { backgroundView.color = PREMIUM_BACKGROUND_COLOR accountTypeIcon.image = resources.getImage(PREMIUM_IMAGE_ID) } Function > Flow > Split by object -
What is wrong with "split by condition" 1/2 Can write
only ambiguous short summary /** * Updates views depending whether the account type is free or premium. */ Function > Flow > Split by object -
What is wrong with "split by condition" 2/2 No guarantee
that all the combinations are covered when (accountType) { PREMIUM -> updateViewsForPremium() FREE -> updateViewsForFree() BUSINESS -> updateViewsForBusiness() } private fun updateViewsForBusiness() { backgroundView.color = PREMIUM_BACKGROUND_COLOR // [BUG] Forgot to update the account icon Function > Flow > Split by object -
Split by object: Good example 1/2 fun bindViewData(accountType: AccountType) {
updateBackgroundViewColor(accountType) updateAccountTypeImage(accountType) } private fun updateBackgroundViewColor(accountType: AccountType) { backgroundView.color = when (accountType) { PREMIUM -> PREMIUM_BACKGROUND_COLOR FREE -> FREE_BACKGROUND_COLOR } } Function > Flow > Split by object -
Split by object: Good example 2/2 Easy to write a
meaningful summary /** * Updates background color and icon according to a given account type. */ Ensures that all the combinations are covered Can conduct further refactoring e.g., Make extracted functions reference transparent Function > Flow > Split by object -
Split by object: Further refactoring fun bindViewData(accountType: AccountType) { backgroundView.color
= getBackgroundColorInt(accountType) accountTypeIcon.image = getAccountTypeIconImage(accountType) } private fun getBackgroundColorInt(accountType: AccountType): Int = when (accountType) { PREMIUM -> PREMIUM_BACKGROUND_COLOR FREE -> FREE_BACKGROUND_COLOR } Function > Flow > Split by object -
Early return VS split by object Does "early return" not
represent "split by condition"? Strategy 1: - Merge unhappy paths into a happy path Strategy 2: - Apply early return first - Apply "split by object" for the happy path Function > Flow > Split by object -
Function flow: Summary Perform definition-based programming: Replace nest/chain/literal with named
variables/functions Focus on a happy path: Apply early return Split by object, not condition: Make a function or block for each target object Function > Flow > Summary -
Summary Check whether it is easy to write a short
summary Function responsibility: - Do not modify when returning a main result Function flow: - Make code understandable by scanning it - Definition-based programming, early return, split by object Function > Summary