Code readability - Speaker Deck

Slide 1

Slide 1 text

Code Readability Munetoshi Ishikawa

Slide 2

Slide 2 text

Code readability session 1 Introduction and Principles

Slide 3

Slide 3 text

What readable code is - Obvious: isVisible rather than flag - Simple: isA && isB rather than !(!isA || !isB) && isB - Isolated: responsibility and dependency - Structured: format, members, interactions, and abstraction layer Introduction and Principles > Introduction

Slide 4

Slide 4 text

Why we need readable code Introduction and Principles > Introduction

Slide 5

Slide 5 text

Why we need readable code Sustainable development - To extend saturation curve of development Introduction and Principles > Introduction

Slide 6

Slide 6 text

Why we need readable code Sustainable development - To extend saturation curve of development Reading code > Writing code - Requesting code review from two or more engineers - Complicated bug ﬁx with a few lines Introduction and Principles > Introduction

Slide 7

Slide 7 text

Optimize for sustainable development Focus on team productivity Introduction and Principles > Introduction

Slide 8

Slide 8 text

Optimize for sustainable development Focus on team productivity Your 5 minutes could save 1 hour for others Add a comment, write test, refactor Introduction and Principles > Introduction

Slide 9

Slide 9 text

Optimize for sustainable development Focus on team productivity Your 5 minutes could save 1 hour for others Add a comment, write test, refactor We may need to update personnel rating criteria Don't focus only on short-term speed to implement Introduction and Principles > Introduction

Slide 10

Slide 10 text

Prisoner's dilemma Team productivity may decrease if we focus on the personal Introduction and Principles > Introduction

Slide 11

Slide 11 text

Prisoner's dilemma Team productivity may decrease if we focus on the personal Eng. A ┃ │ ┃ Write clean code │ Write hacky code Eng. B ┃ │ ━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━━━━ ┃ 8 │ 10 Write clean code ┃ │ ┃ 8 │ 1 ──────────────────╂──────────────────┼─────────────────── ┃ 1 │ 2 Write hacky code ┃ │ ┃ 10 │ 2 Introduction and Principles > Introduction

Slide 12

Slide 12 text

Learn how to write readable code Feature implementation itself is easy Special training is not required Introduction and Principles > Introduction

Slide 13

Slide 13 text

Learn how to write readable code Feature implementation itself is easy Special training is not required No learning, No readable code - Training, lectures - Peer code review - Self-education Introduction and Principles > Introduction

Slide 14

Slide 14 text

Contents of this lecture - Introduction and Principles - Natural language: Naming, Comments - Inner type structure: State, Procedure - Inter type structure: Dependency (two sessions) - Follow-up: Review Introduction and Principles > Introduction

Slide 15

Slide 15 text

Slide 16

Slide 16 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles

Slide 17

Slide 17 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles

Slide 18

Slide 18 text

The boy scout rule Try to leave this world a little better than you found it... — Robert Baden-Powell Introduction and Principles > Principles > The boy scout rule

Slide 19

Slide 19 text

The boy scout rule Try to leave this world a little better than you found it... — Robert Baden-Powell Introduced to software development by Robert C. Martin1 Clean code whenever you have touched it 1 97 Things Every Programmer Should Know: Collective Wisdom from the Experts, Kevlin Henney, 2010 Introduction and Principles > Principles > The boy scout rule

Slide 20

Slide 20 text

Dos for the boy scout rule - Add: comments, tests - Remove: unnecessary dependencies, members, and conditions - Rename: types, functions, and values - Break: huge types, huge functions, nests, and call sequences - Structure: dependencies, abstraction layers, and type hierarchy Introduction and Principles > Principles > The boy scout rule

Slide 21

Slide 21 text

Don'ts for the boy scout rule 1/2 Don't add an element in a huge structure - Sentence in a huge method - Case in a huge conditional branch - Member in a huge type - Callback in a huge call sequence - Inheritance in a huge hierarchy Introduction and Principles > Principles > The boy scout rule

Slide 22

Slide 22 text

Don'ts for the boy scout rule 2/2 Don't add something without thought Introduction and Principles > Principles > The boy scout rule

Slide 23

Slide 23 text

Don'ts for the boy scout rule 2/2 Don't add something without thought Is the code location correct? - Consider restructuring before adding a conditional branch Introduction and Principles > Principles > The boy scout rule

Slide 24

Slide 24 text

Don'ts for the boy scout rule 2/2 Don't add something without thought Is the code location correct? - Consider restructuring before adding a conditional branch Can't you simplify? - Merge copied values or conditions - Look around where you want to change Introduction and Principles > Principles > The boy scout rule

Slide 25

Slide 25 text

Example of "don'ts" 1/2 Question: Is it ﬁne to add a new type Z? val viewType: ViewType = ... // An enum type when (viewType) { A -> { view1.isVisible = true view2.text = "Case A" } B -> { view1.isVisible = false view2.text = "Case B" } ... Introduction and Principles > Principles > The boy scout rule

Slide 26

Slide 26 text

Example of "don'ts" 2/2 Answer: No Introduction and Principles > Principles > The boy scout rule

Slide 27

Slide 27 text

Example of "don'ts" 2/2 Answer: No Don't add a new condition if there are too many branches Introduction and Principles > Principles > The boy scout rule

Slide 28

Slide 28 text

Example of "don'ts" 2/2 Answer: No Don't add a new condition if there are too many branches Solution: Apply strategy pattern Introduction and Principles > Principles > The boy scout rule

Slide 29

Slide 29 text

Example of "dos" 1: Extract parameters as constructor parameters (or computed properties) enum class ViewType(val isView1Visible: Boolean, val view2Text: String) Introduction and Principles > Principles > The boy scout rule

Slide 30

Slide 30 text

Example of "dos" 1: Extract parameters as constructor parameters (or computed properties) enum class ViewType(val isView1Visible: Boolean, val view2Text: String) 2: Remove conditional branches with the parameters view1.isVisible = viewType.isView1Visible view2.text = viewType.view2Text Introduction and Principles > Principles > The boy scout rule

Slide 31

Slide 31 text

Example of "dos" 1: Extract parameters as constructor parameters (or computed properties) enum class ViewType(val isView1Visible: Boolean, val view2Text: String) 2: Remove conditional branches with the parameters view1.isVisible = viewType.isView1Visible view2.text = viewType.view2Text 3: Add a new type Z. Introduction and Principles > Principles > The boy scout rule

Slide 32

Slide 32 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles

Slide 33

Slide 33 text

YAGNI You Aren't Gonna Need It = Implement it only when you need it - 90% of features for the future are not used2 - Keep structure simple = ready for unexpected change2 2 http://www.extremeprogramming.org/rules/early.html Introduction and Principles > Principles > YAGNI

Slide 34

Slide 34 text

YAGNI You Aren't Gonna Need It = Implement it only when you need it - 90% of features for the future are not used2 - Keep structure simple = ready for unexpected change2 Exception: Public library for external people 2 http://www.extremeprogramming.org/rules/early.html Introduction and Principles > Principles > YAGNI

Slide 35

Slide 35 text

YAGNI: Example - Unused types, procedures and values - Interface or abstract class with only one implementation - Function parameter for a constant value - Public global utility function only for one client code - Code commented out Introduction and Principles > Principles > YAGNI

Slide 36

Slide 36 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS

Slide 37

Slide 37 text

KISS Keep It Simple Stupid — Clarence Leonard "Kelly" Johnson Choose simpler solution - Limit and specify the usage of a library/framework/design - Use the default implementation as far as possible Introduction and Principles > Principles > KISS

Slide 38

Slide 38 text

Slide 39

Slide 39 text

KISS: Good example fun getActualDataSingle(): Single> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) Introduction and Principles > Principles > KISS

Slide 40

Slide 40 text

KISS: Good example fun getActualDataSingle(): Single> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single> = Single.just(listOf(1, 10, 100)) Introduction and Principles > Principles > KISS

Slide 41

Slide 41 text

KISS: Bad example 1/2 fun getActualDataSingle(): Single> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single> = Single .fromCallable { listOf(1, 10, 100) } .subscribeOn(ioScheduler) Makes the code complex for unnecessary consistency Introduction and Principles > Principles > KISS

Slide 42

Slide 42 text

KISS: Bad example 2/2 fun getActualDataSingle(): Single> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single> = Observable .range(1, 2) .reduce(listOf(1)) { list, _ -> list + list.last() * 10 } .subscribeOn(ioScheduler) Uses Rx for unnecessary list creation (Rx was used only for changing the thread.) Introduction and Principles > Principles > KISS

Slide 43

Slide 43 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS

Slide 44

Slide 44 text

Single method == small responsibility? Introduction and Principles > Principles > Single responsibility principles

Slide 45

Slide 45 text

Single method == small responsibility? No, deﬁnitely! class Alviss { // May show a text, may break the device, may launch a rocket, // may ... fun doEverything(state: UniverseState) } Introduction and Principles > Principles > Single responsibility principles

Slide 46

Slide 46 text

Single responsibility principle A class should have only one reason to change. — Robert C. Martin Introduction and Principles > Principles > Single responsibility principles

Slide 47

Slide 47 text

Single responsibility principle A class should have only one reason to change. — Robert C. Martin We should not mix up two unrelated features Introduction and Principles > Principles > Single responsibility principles

Slide 48

Slide 48 text

Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList, val bookNames: MutableList, ) { ... Introduction and Principles > Principles > Single responsibility principles

Slide 49

Slide 49 text

Slide 50

Slide 50 text

Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList, val bookNames: MutableList, val bookIdToRenterNameMap: MutableMap, val bookIdToDueDateMap: MutableMap, ... ) { ... Introduction and Principles > Principles > Single responsibility principles

Slide 51

Slide 51 text

Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList, val bookNames: MutableList, val bookIdToRenterNameMap: MutableMap, val bookIdToDueDateMap: MutableMap, ... ) { fun findRenterName(bookName: String): String? fun findDueDate(bookName: String): Date? ... Introduction and Principles > Principles > Single responsibility principles

Slide 52

Slide 52 text

Single responsibility principle: What is wrong Introduction and Principles > Principles > Single responsibility principles

Slide 53

Slide 53 text

Single responsibility principle: What is wrong BookRentalData has all data of book, user, and circulation record Introduction and Principles > Principles > Single responsibility principles

Slide 54

Slide 54 text

Single responsibility principle: What is wrong BookRentalData has all data of book, user, and circulation record Split a model class for each entity Introduction and Principles > Principles > Single responsibility principles

Slide 55

Slide 55 text

Single responsibility principle: Good example data class BookData(val id: Int, val name: String, ...) data class UserData(val name: String, ...) Introduction and Principles > Principles > Single responsibility principles

Slide 56

Slide 56 text

Single responsibility principle: Good example data class BookData(val id: Int, val name: String, ...) data class UserData(val name: String, ...) class CirculationRecord( val onLoanBookEntries: MutableMap ) { data class Entry(val renter: UserData, val dueDate: Date) Introduction and Principles > Principles > Single responsibility principles

Slide 57

Slide 57 text

Keep responsibility small Split types - Model for each entity - Logic for each layer and component - Utility for each target type Introduction and Principles > Principles > Single responsibility principles

Slide 58

Slide 58 text

How to conﬁrm responsibility List what the type does, and try to summarize it Introduction and Principles > Principles > Single responsibility principles

Slide 59

Slide 59 text

How to conﬁrm responsibility List what the type does, and try to summarize it Split the type for each case as follows - It's hard to make a summary - The summary is too fat compared with the name Introduction and Principles > Principles > Single responsibility principles

Slide 60

Slide 60 text

Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS

Slide 61

Slide 61 text

Premature optimization 1/2 We should forget about small efﬁciencies, say about 97% of the time: premature optimization is the root of all evil. — Structured Programming with go to Statements, Donald Knuth Introduction and Principles > Principles > Premature optimization

Slide 62

Slide 62 text

Premature optimization 2/2 Good: if the optimization cleans the code Bad: if the optimization makes the code complex Introduction and Principles > Principles > Premature optimization

Slide 63

Slide 63 text

Example of good optimization 1/2 Before: val data = arrayList.firstOrNull { data -> data.key == expectedKey } Introduction and Principles > Principles > Premature optimization

Slide 64

Slide 64 text

Example of good optimization 1/2 Before: val data = arrayList.firstOrNull { data -> data.key == expectedKey } After: val data = hashMap.getOrNull(key) Introduction and Principles > Principles > Premature optimization

Slide 65

Slide 65 text

Example of good optimization 1/2 Before: val data = arrayList.firstOrNull { data -> data.key == expectedKey } After: val data = hashMap.getOrNull(key) Simplify the code while reducing the calculation cost Introduction and Principles > Principles > Premature optimization

Slide 66

Slide 66 text

Examples of bad optimization Don't optimize without proﬁling or platform support, either - Mutable instance reusing - Lazy initialization - Cache - Inline extraction - Instance pool Introduction and Principles > Principles > Premature optimization

Slide 67

Slide 67 text

Drawbacks of optimization - May obstruct simpliﬁcation - Compiler is sometimes smarter than us - May require overhead cost - Lazy initialization: (Synchronized) instance check - Cache: cache miss ratio * cache access time Introduction and Principles > Principles > Premature optimization

Slide 68

Slide 68 text

Required actions before optimization - Ask yourself "Do I really need it?" or "Is it easy enough to implement?" - Proﬁle Target (time, memory), number, rate Introduction and Principles > Principles > Premature optimization

Slide 69

Slide 69 text

Summary Focus on sustainable development: Readability Introduction and Principles > Summary

Slide 70

Slide 70 text

Summary Focus on sustainable development: Readability The boy scout rule: Clean code whenever you have touched it Introduction and Principles > Summary

Slide 71

Slide 71 text

Summary Focus on sustainable development: Readability The boy scout rule: Clean code whenever you have touched it YAGNI: Implement only when you need it Introduction and Principles > Summary

Slide 72

Slide 72 text

Slide 73

Slide 73 text

Summary Focus on sustainable development: Readability The boy scout rule: Clean code whenever you have touched it YAGNI: Implement only when you need it KISS: Use simple method, beautiful != readable Single responsibility principle: Make the scope clear Introduction and Principles > Summary

Slide 74

Slide 74 text

Slide 75

Slide 75 text

codeReadabilitySession2 N_A_M_I_N_G

Slide 76

Slide 76 text

Slide 77

Slide 77 text

What we name - Type: class, interface, enum, struct, protocol, trait - Value: property, ﬁeld, parameter, local value - Procedure: function, method, subroutine - Scope: package, module, namespace - Resource: ﬁle, directory, ID - etc. Naming > Introduction

Slide 78

Slide 78 text

What a good name is - Accurate isVisible shouldn't be used for sound - Descriptive width/height rather than w/h - Unambiguous imageView/imageBitmap/imageUrl rather than image Naming > Introduction

Slide 79

Slide 79 text

How to create a good name - Use correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit sufﬁx - Use positive afﬁrmation Naming > Introduction

Slide 80

Slide 80 text

Topics - Use correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit sufﬁx - Use positive afﬁrmation Naming > Introduction

Slide 81

Slide 81 text

Why grammar is important Question 1: What is ListenerEventMessageClickViewText? Naming > Use the correct grammar

Slide 82

Slide 82 text

Why grammar is important Question 1: What is ListenerEventMessageClickViewText? Answer?: A text of a click view (?) of listener event message (???) Naming > Use the correct grammar

Slide 83

Slide 83 text

Why grammar is important Question 1: What is ListenerEventMessageClickViewText? Answer?: A text of a click view (?) of listener event message (???) Question 2: What is MessageTextViewClickEventListener? Naming > Use the correct grammar

Slide 84

Slide 84 text

Slide 85

Slide 85 text

Use correct grammar "Grammar" depends on the language and convention Let's look at the case of Java/Kotlin Naming > Use the correct grammar

Slide 86

Slide 86 text

Types of names 1/2 - Noun: Type, value (including property function) imageView, HashSet, indexOf - Imperative: Procedure findMessage, compareTo Naming > Use the correct grammar

Slide 87

Slide 87 text

Types of names 2/2 - Adjective, participle: Interface, state type/constant Iterable, PLAYING, CONNECTED - Interrogative, third-person verb: Boolean value/function isTextVisible, contains, equalsTo, may/shouldShow - Adverb phrase with preposition: Converter, callback toInt, fromMemberId, onNewIntent Naming > Use the correct grammar

Slide 88

Slide 88 text

Grammar: Nouns Place the essential words at the last (= the word telling what it is) Naming > Use the correct grammar

Slide 89

Slide 89 text

Grammar: Nouns Place the essential words at the last (= the word telling what it is) ! : MessageEventHandler, buttonHeight Naming > Use the correct grammar

Slide 90

Slide 90 text

Grammar: Nouns Place the essential words at the last (= the word telling what it is) ! : MessageEventHandler, buttonHeight ! : xyzHeightForPortlait (don't use for a type name) Naming > Use the correct grammar

Slide 91

Slide 91 text

Grammar: Nouns Place the essential words at the last (= the word telling what it is) ! : MessageEventHandler, buttonHeight ! : xyzHeightForPortlait (don't use for a type name) ! : HandlerMessageEvent (if it's a handler), heightPortlait Naming > Use the correct grammar

Slide 92

Slide 92 text

Slide 93

Slide 93 text

Grammar: Imperative Place a verb at the ﬁrst Naming > Use the correct grammar

Slide 94

Slide 94 text

Grammar: Imperative Place a verb at the ﬁrst "get X": ! getX, " xGet Naming > Use the correct grammar

Slide 95

Slide 95 text

Grammar: Imperative Place a verb at the ﬁrst "get X": ! getX, " xGet "post Y": ! postY, " yPost Naming > Use the correct grammar

Slide 96

Slide 96 text

Grammar: Imperative Place a verb at the ﬁrst "get X": ! getX, " xGet "post Y": ! postY, " yPost "map to Z": ! mapToZ, " toZMap, zToMap Naming > Use the correct grammar

Slide 97

Slide 97 text

How a name in a wrong order is created class UserActionEvent Naming > Use the correct grammar

Slide 98

Slide 98 text

How a name in a wrong order is created class UserActionEvent class UserActionEventSwipe: UserActionEvent() class UserActionEventClick: UserActionEvent() Naming > Use the correct grammar

Slide 99

Slide 99 text

How a name in a wrong order is created class UserActionEvent class UserActionEventSwipe: UserActionEvent() class UserActionEventClickMessageText: UserActionEvent() class UserActionEventClickProfileImage: UserActionEvent() Naming > Use the correct grammar

Slide 100

Slide 100 text

Slide 101

Slide 101 text

Grammar: Summary - Types for a name Noun, imperative, adjective, interrogative (+ third person), adverb - Word order is important Imagine how the name looks on the caller side Naming > Use the correct grammar

Slide 102

Slide 102 text

Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit sufﬁx - Use positive afﬁrmation Naming > Describe what rather than who/when

Slide 103

Slide 103 text

Describe what rather than who/when - A name must answer ! What a type/value is ! What a procedure does Naming > Describe what rather than who/when

Slide 104

Slide 104 text

Describe what rather than who/when - A name must answer ! What a type/value is ! What a procedure does - A name should not mention to the caller " Who it calls/uses " When/Where/Why/How it's called/used Naming > Describe what rather than who/when

Slide 105

Slide 105 text

Function name example: Declaration Good ! : Describe what this procedure does class MessageRepository { fun storeReceivedMessage(data: MessageData) { Naming > Describe what rather than who/when

Slide 106

Slide 106 text

Function name example: Declaration Good ! : Describe what this procedure does class MessageRepository { fun storeReceivedMessage(data: MessageData) { Bad ! : Describe when this procedure should be called class MessageRepository { fun onMessageReceived(data: MessageData) { Naming > Describe what rather than who/when

Slide 107

Slide 107 text

Slide 108

Slide 108 text

Function name example: Caller code Good ! : We can know what happens by the calling code repository.storeReceivedMessage(messageData) handler.post { presenter.showNewReceivedMessage(messageData) } Bad ! : We can't know what happens repository.onMessageReceived(messageData) handler.post { presenter.onMessageReceived(messageData) } Naming > Describe what rather than who/when

Slide 109

Slide 109 text

Function name example: Another bad reason The procedure responsibility is ambiguous Naming > Describe what rather than who/when

Slide 110

Slide 110 text

Slide 111

Slide 111 text

Slide 112

Slide 112 text

Function name example: Another bad reason The procedure responsibility is ambiguous class MessageViewPresenter { fun onMessageReceived(data: MessageData) { // View presentation code for new message ... launch(...) { repository.onMessageReceived(data) } // !! What happens with the following code? repository.onMessageReceived(messageData) handler.post { presenter.onMessageReceived(messageData) } Naming > Describe what rather than who/when

Slide 113

Slide 113 text

Parameter name example: Declaration Good ! : We can know what happens if it's true fun showHistory(shouldShowDialogOnError: Boolean) Naming > Describe what rather than who/when

Slide 114

Slide 114 text

Parameter name example: Declaration Good ! : We can know what happens if it's true fun showHistory(shouldShowDialogOnError: Boolean) Bad ! : We can't know what happens if it's true fun showHistory(isCalledFromMainActivity: Boolean) Naming > Describe what rather than who/when

Slide 115

Slide 115 text

Parameter name example: Reason of "bad" 1: The name easily becomes obsolete // ... from another activity requiring dialog showHistory(isCalledFromMainActivity = true) Naming > Describe what rather than who/when

Slide 116

Slide 116 text

Parameter name example: Reason of "bad" 1: The name easily becomes obsolete // ... from another activity requiring dialog showHistory(isCalledFromMainActivity = true) 2: The parameter will be used for other purposes if (isCalledFromMainActivity) { setActivityResult(...) Naming > Describe what rather than who/when

Slide 117

Slide 117 text

Slide 118

Slide 118 text

Describe what: Exception May need to name by how/when for an abstract callback interface - e.g., onClicked / onSwiped / onDestroyed Naming > Describe what rather than who/when

Slide 119

Slide 119 text

Slide 120

Slide 120 text

Describe what: Exception May need to name by how/when for an abstract callback interface - e.g., onClicked / onSwiped / onDestroyed - Because "what" is not decided on the declaration We should name by "what" if possible even for a callback abstract fun toggleSelectionState() ... view.setOnClickListener { toggleSelectionState() } Naming > Describe what rather than who/when

Slide 121

Slide 121 text

Describe what: Summary Describe "what" it does/is = Should not mention to the caller (who/when/where/why/how) Naming > Describe what rather than who/when

Slide 122

Slide 122 text

Describe what: Summary Describe "what" it does/is = Should not mention to the caller (who/when/where/why/how) - Show code responsibility clearly - Make caller code readable Naming > Describe what rather than who/when

Slide 123

Slide 123 text

Describe what: Summary Describe "what" it does/is = Should not mention to the caller (who/when/where/why/how) - Show code responsibility clearly - Make caller code readable Exception: Abstract callback interface Naming > Describe what rather than who/when

Slide 124

Slide 124 text

Slide 125

Slide 125 text

Ambiguous words: Example 1/2 Question: What does initializationFlag represent? - shouldInitialize - isInitializing - is/wasInitialized - isInitializable - isNotInitialized (!!) Naming > Choose unambiguous words

Slide 126

Slide 126 text

Slide 127

Slide 127 text

Ambiguous words: Example 2/2 Question: What does sizeLimit represent? - max or min? - height, width, byte, characters, or length? Naming > Choose unambiguous words

Slide 128

Slide 128 text

Ambiguous words: Example 2/2 Question: What does sizeLimit represent? - max or min? - height, width, byte, characters, or length? Name maxHeight, for example. Naming > Choose unambiguous words

Slide 129

Slide 129 text

Rewording options to consider - ﬂag: is, was, should, can, may, will ... - check: is, query, verify, measure, filter, notify, update ... - good, ﬁne: valid, completed, reliable, secure, satisfies ... - old: previous, stored, expired, invalidated, deprecated ... - tmp, retval: actual name Naming > Choose unambiguous words

Slide 130

Slide 130 text

Slide 131

Slide 131 text

Choose unambiguous words: Summary - Avoid words like flag and check - Use a dictionary and thesaurus Naming > Choose unambiguous words

Slide 132

Slide 132 text

Slide 133

Slide 133 text

Abbreviations: Example 1/2 Question: What does im stand for? Naming > Avoid confusing abbreviations

Slide 134

Slide 134 text

Abbreviations: Example 1/2 Question: What does im stand for? input method, illegal message, instance manager ... Naming > Avoid confusing abbreviations

Slide 135

Slide 135 text

Abbreviations: Example 1/2 Question: What does im stand for? input method, illegal message, instance manager ... Don't use your own acronyms - Recognizing is easier than recalling3 3 100 Things: Every Designer Needs to Know About People, Susan Weinschenk, 2011 Naming > Avoid confusing abbreviations

Slide 136

Slide 136 text

Abbreviations: Example 2/2 Question: What does str stand for? Naming > Avoid confusing abbreviations

Slide 137

Slide 137 text

Abbreviations: Example 2/2 Question: What does str stand for? string, structure, stream, streak, street, sorted transaction record Naming > Avoid confusing abbreviations

Slide 138

Slide 138 text

Abbreviations: Example 2/2 Question: What does str stand for? string, structure, stream, streak, street, sorted transaction record Abbreviations may be acceptable if commonly used - Abbreviations like URL and TCP are totally ﬁne - str for string is acceptable especially for a limited scope Naming > Avoid confusing abbreviations

Slide 139

Slide 139 text

Project-speciﬁc abbreviation Some abbreviations are used project-wide Naming > Avoid confusing abbreviations

Slide 140

Slide 140 text

Project-speciﬁc abbreviation Some abbreviations are used project-wide Make readable for a new team member - Write documentation for type, value, or procedure - Prepare glossary Naming > Avoid confusing abbreviations

Slide 141

Slide 141 text

Avoid confusing abbreviations: Summary - Don't use your own abbreviations - Commonly used abbreviations are acceptable - Prepare document or glossary for project-speciﬁc abbreviations Naming > Avoid confusing abbreviations

Slide 142

Slide 142 text

Slide 143

Slide 143 text

Add type/unit suffix Add suffix as a supplement type/unit - timeout: timeoutMillis, timeoutHours - width: widthPoints, widthPx, widthInches - color: colorInt, colorResId - i, j, k: xxxIndex, row, col Naming > Add type/unit suffix

Slide 144

Slide 144 text

Add type/unit sufﬁx: Aside Consider creating a wrapper class of a unit Naming > Add type/unit sufﬁx

Slide 145

Slide 145 text

Add type/unit sufﬁx: Aside Consider creating a wrapper class of a unit class Inch(val value: Int) class Centimeter(val value: Int) Naming > Add type/unit sufﬁx

Slide 146

Slide 146 text

Add type/unit sufﬁx: Aside Consider creating a wrapper class of a unit class Inch(val value: Int) class Centimeter(val value: Int) fun setWidth(width: Inch) = ... setWidth(Centimeter(10)) // Compile error! Naming > Add type/unit sufﬁx

Slide 147

Slide 147 text

Slide 148

Slide 148 text

Slide 149

Slide 149 text

Use positive afﬁrmation ! : Positive words, isEnabled Naming > Use positive afﬁrmation

Slide 150

Slide 150 text

Use positive afﬁrmation ! : Positive words, isEnabled ! : Negative words, isDisabled Naming > Use positive afﬁrmation

Slide 151

Slide 151 text

Use positive afﬁrmation ! : Positive words, isEnabled ! : Negative words, isDisabled ! : Positive words with "not", "no", or "non", isNotEnabled Naming > Use positive afﬁrmation

Slide 152

Slide 152 text

Use positive afﬁrmation ! : Positive words, isEnabled ! : Negative words, isDisabled ! : Positive words with "not", "no", or "non", isNotEnabled ! : Negative words with "not", "no", or "non", isNotDisabled Naming > Use positive afﬁrmation

Slide 153

Slide 153 text

Slide 154

Slide 154 text

Slide 155

Slide 155 text

Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit sufﬁx - Use positive afﬁrmation - Adhere to language/platform/project conventions Naming > Adhere to language/platform/project conventions

Slide 156

Slide 156 text

Language/platform/project conventions Adhere to convention/rule/manner of language/platform/project Naming > Adhere to language/platform/project conventions

Slide 157

Slide 157 text

Language/platform/project conventions Adhere to convention/rule/manner of language/platform/project Some conventions use a get... method, satisfying the following requirements - Returns a value - Finishes immediately and O(1) - Has no side-effect Naming > Adhere to language/platform/project conventions

Slide 158

Slide 158 text

Summary A name must be accurate, descriptive, and unambiguous Naming > Summary

Slide 159

Slide 159 text

Summary A name must be accurate, descriptive, and unambiguous Pay attention to - describing "what" rather than "who/when" - grammar and word choice Naming > Summary

Slide 160

Slide 160 text

/** Code readability session 3 */ // Comments

Slide 161

Slide 161 text

Slide 162

Slide 162 text

Why we should write comments Readability - Convey intent: summarize, explain reasoning - Prevent mistakes: note precautions - Refactor: rename, simplify Comments > Introduction

Slide 163

Slide 163 text

Why we should write comments Readability - Convey intent: summarize, explain reasoning - Prevent mistakes: note precautions - Refactor: rename, simplify We don't need a comment if the code is simple enough (depending on the convention) Comments > Introduction

Slide 164

Slide 164 text

Slide 165

Slide 165 text

Refactoring with comments: Example /** * Adds a new pair of keyword and the definition to this dictionary, * can be referenced by [getDefinition(String)]. * * If the keyword is already registered, this registration fails. * Then, this returns a boolean representing whether the registration * succeeded. */ fun add(newData: Pair): Boolean Comments > Introduction

Slide 166

Slide 166 text

Refactoring with comments: Refactoring plan Think about why we need a long comment Comments > Introduction

Slide 167

Slide 167 text

Refactoring with comments: Refactoring plan Think about why we need a long comment for the parameter, error case, and return value - Rename method - Break parameters - Simplify error cases - Remove unnecessary return value Comments > Introduction

Slide 168

Slide 168 text

Refactoring after comment: Refactoring result /** * Adds or overwrites a definition of a given [keyword]. * The registered definition is obtained by [getDefinition(String)]. */ fun registerDefinition(keyword: String, definitionText: String) Comments > Introduction

Slide 169

Slide 169 text

Types of comments - Documentation: Formal comment for type/value/procedure... - Inline comment: Informal comment in a code block - To do: // TODO: , // FIXME: - IDE/compiler support, code generation: // $COVERAGE-IGNORE$ Comments > Introduction

Slide 170

Slide 170 text

Topics - Documentation - Inline comment Comments > Introduction

Slide 171

Slide 171 text

Topics - Documentation - Inline comment Comments > Documentation

Slide 172

Slide 172 text

What we write documentation for - Type: class, struct, interface, protocol, enum ... - Value: ﬁeld, property, constant, parameter ... - Procedure: global function, method, extension ... - Scope: package, module, namespace ... Comments > Documentation

Slide 173

Slide 173 text

Format of documentation Starts with... - `/**` for Kotlin, Java, Swift and Objective-C - `///` for Swift and Objective-C - `/*!` for Objective-C Refer to documentation for KDoc, JavaDoc, Swift Markup or HeaderDoc for more details Comments > Documentation

Slide 174

Slide 174 text

Contents of documentation Let's take a look at anti-patterns ﬁrst Comments > Documentation

Slide 175

Slide 175 text

Anti-patterns: Auto-generated comment /** * @param keyword * @return */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns

Slide 176

Slide 176 text

Anti-patterns: Same to the name /** * Gets the description for a keyword. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns

Slide 177

Slide 177 text

Anti-patterns: Translation of the code /** * Calls [doA] if `conditionA` is satisfied. * Otherwise, calls [doB] and if ... */ fun getDescription(keyword: String): String { if (conditionA) { doA() } else { doB() if (...) {...} } Comments > Documentation > Anti-patterns

Slide 178

Slide 178 text

Anti-patterns: Referring to private members /** * Returns a string stored in a private map [dictionary]. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns

Slide 179

Slide 179 text

Anti-patterns: No summary /** * Throws an exception if the given `keyword` is empty. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns

Slide 180

Slide 180 text

Anti-patterns: Mentioning to callers /** * ... * This is called by class [UserProfilePresenter]. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns

Slide 181

Slide 181 text

Lessons from anti-patterns Write documentation only when explanation is required - Fine to skip if type/value/procedure name is enough Comments > Documentation > Anti-patterns

Slide 182

Slide 182 text

Lessons from anti-patterns Write documentation only when explanation is required - Fine to skip if type/value/procedure name is enough Summarize "what it is/does" - Without mentioning to callers - Without using implementation details Comments > Documentation > Anti-patterns

Slide 183

Slide 183 text

Contents of documentation Comments > Documentation > Overview

Slide 184

Slide 184 text

Contents of documentation Short summary (Mandatory) "What it is/does" in a phrase/sentence Comments > Documentation > Overview

Slide 185

Slide 185 text

Contents of documentation Short summary (Mandatory) "What it is/does" in a phrase/sentence Details (Optional) Additional sentences to explain usages, limitations, and so on Comments > Documentation > Overview

Slide 186

Slide 186 text

Slide 187

Slide 187 text

Short summary: How to write 1/2 Find the most important line/block/element of code Comments > Documentation > Short summary

Slide 188

Slide 188 text

Short summary: How to write 1/2 Find the most important line/block/element of code if (!user.isValid) { return } val rawProfileImage = getProfileImage(user.id, ...) val roundProfileImage = applyRoundFilter(rawProfileImage, ...) profileView.setImage(roundProfileImage) Comments > Documentation > Short summary

Slide 189

Slide 189 text

Slide 190

Slide 190 text

Short summary: How to write 2/2 Complement information based on the most important point Comments > Documentation > Short summary

Slide 191

Slide 191 text

Short summary: How to write 2/2 Complement information based on the most important point /** * Shows ... a profile image * */ ... profileView.setImage(roundProfileImage) Comments > Documentation > Short summary

Slide 192

Slide 192 text

Short summary: How to write 2/2 Complement information based on the most important point /** * Shows a roundly cut profile image of a given [user]. * ... */ ... profileView.setImage(roundProfileImage) Comments > Documentation > Short summary

Slide 193

Slide 193 text

Slide 194

Slide 194 text

Convention of short summary Refer to standard library documents Examples: Kotlin, Java, Swift, Objective-C - Type, Value: Starts with a noun phrase e.g., "A generic ordered collection of elements."4 - Procedure: Starts with a verb with 3rd-person singular form e.g., "Adds a new element at the end of the array."5 5 https://developer.apple.com/documentation/swift/array/3126937-append 4 https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/#list Comments > Documentation > Short summary

Slide 195

Slide 195 text

Slide 196

Slide 196 text

Contents of "details" in documentation - Speciﬁcation and usage - Function return value - Limitation and error values/status - Examples - ... Comments > Documentation > Details

Slide 197

Slide 197 text

Contents of "details" in documentation - Speciﬁcation and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details

Slide 198

Slide 198 text

Speciﬁcation and usage Comment on typical usage and expected behavior Comments > Documentation > Details

Slide 199

Slide 199 text

Speciﬁcation and usage Comment on typical usage and expected behavior /** * ... * To update view components such as message text and sender name, * give [MessageData] model object to [bindView]. */ class MessageViewPresenter(messageView: View) Comments > Documentation > Details

Slide 200

Slide 200 text

Contents of "details" in documentation - Speciﬁcation and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details

Slide 201

Slide 201 text

Function return value 1/2 Question: What does the following return value mean? fun setSelectedState(isSelected: Boolean): Boolean Comments > Documentation > Details

Slide 202

Slide 202 text

Slide 203

Slide 203 text

Function return value 1/2 Question: What does the following return value mean? fun setSelectedState(isSelected: Boolean): Boolean Possible answer: isToggled // true if the state is changed wasSelected // previous state before the function call Comments > Documentation > Details

Slide 204

Slide 204 text

Slide 205

Slide 205 text

Slide 206

Slide 206 text

Function return value 2/2 Comment on a return value if the function name is not enough - has side effects for function call - has contract on the return value Comments > Documentation > Details

Slide 207

Slide 207 text

Function return value 2/2 Comment on a return value if the function name is not enough - has side effects for function call - has contract on the return value /** * ... * ... returns true if it was selected before this function call. */ fun setSelectedState(isSelected: Boolean): Boolean Comments > Documentation > Details

Slide 208

Slide 208 text

Function return value 2/2 Comment on a return value if the function name is not enough - has side effects for function call - has contract on the return value /** * ... * The profile ID is non-negative value. */ fun getProfileId(): Int Comments > Documentation > Details

Slide 209

Slide 209 text

Contents of "details" in documentation - Speciﬁcation and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details

Slide 210

Slide 210 text

Limitation and error values/status Comment on the expected precondition and error values/status - A required receiver state to call a function - Restrictions on the arguments Comments > Documentation > Details

Slide 211

Slide 211 text

Limitation and error values/status Comment on the expected precondition and error values/status - A required receiver state to call a function - Restrictions on the arguments /** * ... * [prepare] must be called before calling [play] or [seek], * or this throws [ResourceNotReadyException]. */ class VideoPlayer(videoPath: String) Comments > Documentation > Details

Slide 212

Slide 212 text

Limitation and error values/status Comment on the expected precondition and error values/status - A required receiver state to call a function - Restrictions on the arguments /** * ... * Returns `null` if the given `position` is out of the array range. */ fun valueAt(position: Int): T? Comments > Documentation > Details

Slide 213

Slide 213 text

Limitations: Example - Arguments and receiver state - Instance lifecycle - Caller thread - Reentrancy, idempotency ... - Calculation cost, memory size - External environment (e.g., network, database) Comments > Documentation > Details

Slide 214

Slide 214 text

Contents of "details" in documentation - Speciﬁcation and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details

Slide 215

Slide 215 text

Examples Make code more understandable with examples - Example code for usage - Parameter or return value example /** * ... * For example, this returns `arrayOf("a", "bc", "", "d")` * for argument `"a, bc ,,d"` */ fun splitByComma(string: String): Array {... Comments > Documentation > Details

Slide 216

Slide 216 text

Documentation: Summary Comments > Documentation > Summary

Slide 217

Slide 217 text

Documentation: Summary Target: Type, value, procedure, and scope Comments > Documentation > Summary

Slide 218

Slide 218 text

Documentation: Summary Target: Type, value, procedure, and scope Contents: Short summary (mandatory), details (optional) Comments > Documentation > Summary

Slide 219

Slide 219 text

Documentation: Summary Target: Type, value, procedure, and scope Contents: Short summary (mandatory), details (optional) Cautions: - Write only when the name is insufﬁcient as explanation - Don't mention to the caller or the implementation detail Comments > Documentation > Summary

Slide 220

Slide 220 text

Topics - Documentation - Inline comment Comments > Inline comment

Slide 221

Slide 221 text

What inline comment is: Kotlin case Comments > Inline comment

Slide 222

Slide 222 text

What inline comment is: Kotlin case // Function explanation, which won't appear on KDoc fun function(param /* Parameter explanation */: Param) { // Code block summary someMethod(value) // Reason of a statement newId = param.id + 123 /* Reason of constant value */ + ... ... Comments > Inline comment

Slide 223

Slide 223 text

Slide 224

Slide 224 text

What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment

Slide 225

Slide 225 text

What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment

Slide 226

Slide 226 text

Inline comment for large code Make code blocks with comments to summarize what they do Comments > Inline comment

Slide 227

Slide 227 text

Inline comment for large code Make code blocks with comments to summarize what they do ... val messageKey = ... val messageData = messageCache[messageKey] ... if (messageData != null || ...) { // <- When this satisfies? ... // <- Hard to overview code in a nest } Comments > Inline comment

Slide 228

Slide 228 text

Inline comment for large code Make code blocks with comments to summarize what they do // Get message data cache if it's available val messageKey = ... val messageData = messageCache[messageKey] ... // Load message data from DB if there's no cached data. if (messageData != null || ...) { ... } Comments > Inline comment

Slide 229

Slide 229 text

What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment

Slide 230

Slide 230 text

Inline comment for unintuitive code Explain summary/reason if it's hard to understand // // wordReplacementData.reverse() .forEach { (startIndex, endIndex, newText) -> stringBuilder.replace(startIndex, endIndex, newText) } Comments > Inline comment

Slide 231

Slide 231 text

Slide 232

Slide 232 text

Inline comment for unintuitive code Explain summary/reason if it's hard to understand // Replace texts in the reverse order because `replace()` // affects the following indices. wordReplacementData.reverse() .forEach { (startIndex, endIndex, newText) -> stringBuilder.replace(startIndex, endIndex, newText) } Comments > Inline comment

Slide 233

Slide 233 text

Slide 234

Slide 234 text

What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment

Slide 235

Slide 235 text

Inline comment for workaround Explain what a workaround does and explain the reason // We restore the previous state here // because libraryFunction() may break the receiver state Comments > Inline comment

Slide 236

Slide 236 text

Slide 237

Slide 237 text

Inline comments: Summary Essentially the same with documentation - Short summary is not mandatory Comments > Inline comment

Slide 238

Slide 238 text

Inline comments: Summary Essentially the same with documentation - Short summary is not mandatory Explain for large, complex, unintuitive code - Code block, summary, reason, and links Comments > Inline comment

Slide 239

Slide 239 text

Summary - Comment with whatever readers require - Refactor before/after writing comments - Documentation: Short summary (mandatory) and details (optional) - Inline comments: Explanation for large, complex, unintuitive code Comments > Summary

Slide 240

Slide 240 text

const val CODE_READABILITY_SESSION_4 var State

Slide 241

Slide 241 text

Slide 242

Slide 242 text

Object state and readability 1/2 It's hard to read overly stated code State > Introduction

Slide 243

Slide 243 text

Object state and readability 1/2 It's hard to read overly stated code Question: Reference transparent/immutable == readable? State > Introduction

Slide 244

Slide 244 text

Object state and readability 1/2 It's hard to read overly stated code Question: Reference transparent/immutable == readable? No, sometimes it can be easier to read code with side-effect or mutable state. State > Introduction

Slide 245

Slide 245 text

Object state and readability 2/2 Let's look at an example of width ﬁrst search of a binary tree class Node(value: Int, left: Node?, right: Node?) State > Introduction

Slide 246

Slide 246 text

Object state and readability 2/2 Let's look at an example of width ﬁrst search of a binary tree class Node(value: Int, left: Node?, right: Node?) Case1: With a mutable queue Case2: With recursive call State > Introduction

Slide 247

Slide 247 text

Case1: With a mutable queue fun search(valueToFind: Int, root: Node): Node? { val queue = ArrayDeque() var node: Node? = root while (node != null && node.value != valueToFind) { node.left?.let(queue::add) node.right?.let(queue::add) node = queue.poll() } return node } State > Introduction

Slide 248

Slide 248 text

Case2: With recursive call fun search(valueToFind: Int, root: Node): Node? = innerSearch(valueToFind, listOf(root)) tailrec fun innerSearch(valueToFind: Int, queue: List): Node? { val node = queue.getOrNull(0) if (node == null || node.value == valueToFind) { return node } val nextQueue = queue.subList(1, queue.size) + (node.left?.let(::listOf) ?: emptyList()) + (node.right?.let(::listOf) ?: emptyList()) return innerSearch(valueToFind, nextQueue) } State > Introduction

Slide 249

Slide 249 text

Problems of the recursive call example State > Introduction

Slide 250

Slide 250 text

Problems of the recursive call example More complicated - Required a separate public interface from recursive function - Complex queue calculation for the next iteration State > Introduction

Slide 251

Slide 251 text

Problems of the recursive call example More complicated - Required a separate public interface from recursive function - Complex queue calculation for the next iteration No guarantee that valueToFind is constant - Recursive call arguments can be changed for each call - A nested function is required to make it constant State > Introduction

Slide 252

Slide 252 text

What we should care about State > Introduction

Slide 253

Slide 253 text

What we should care about Fold/recursive call is just a way to make code readable/robust - Must not be an objective - Apply fold/recursive call only when it makes code readable State > Introduction

Slide 254

Slide 254 text

What we should care about Fold/recursive call is just a way to make code readable/robust - Must not be an objective - Apply fold/recursive call only when it makes code readable Focus on actual program state rather than apparent immutability - Mutability within a small scope may be ﬁne - Fold/recursive call may adversely affect actual state State > Introduction

Slide 255

Slide 255 text

Topics How to simplify states for readability State > Introduction

Slide 256

Slide 256 text

Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > Introduction

Slide 257

Slide 257 text

Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > Orthogonality

Slide 258

Slide 258 text

"Orthogonal" relationship: Deﬁnition State > Orthogonality

Slide 259

Slide 259 text

"Orthogonal" relationship: Deﬁnition For two properties, they are "orthogonal" if one is modiﬁable regardless of the other's value State > Orthogonality

Slide 260

Slide 260 text

"Orthogonal" relationship: Example Orthogonal: authorName and layoutVisibility Both properties can be updated regardless of the other value State > Orthogonality

Slide 261

Slide 261 text

"Orthogonal" relationship: Example Orthogonal: authorName and layoutVisibility Both properties can be updated regardless of the other value Non-orthogonal: authorId and authorName Both properties depend on each other authorName must be updated when authorId is also updated State > Orthogonality

Slide 262

Slide 262 text

Why we avoid non-orthogonal relationships Inconsistent state might occur State > Orthogonality

Slide 263

Slide 263 text

Why we avoid non-orthogonal relationships Inconsistent state might occur var coinCount: Int var coinText: String State > Orthogonality

Slide 264

Slide 264 text

Why we avoid non-orthogonal relationships Inconsistent state might occur var coinCount: Int var coinText: String What does coinCount == 10 && coinText == "0 coin" represent? State > Orthogonality

Slide 265

Slide 265 text

How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality

Slide 266

Slide 266 text

How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality

Slide 267

Slide 267 text

Replace by using getter/function 1/2 Non-orthogonal properties var coinCount: Int var coinText: String State > Orthogonality > Remove with getter/function

Slide 268

Slide 268 text

Replace by using getter/function 1/2 Non-orthogonal properties var coinCount: Int var coinText: String Remove variable by means of a property getter State > Orthogonality > Remove with getter/function

Slide 269

Slide 269 text

Replace by using getter/function 2/2 Create coinText value by coinCount var coinCount: Int var coinText: String State > Orthogonality > Remove with getter/function

Slide 270

Slide 270 text

Replace by using getter/function 2/2 Create coinText value by coinCount var coinCount: Int val coinText: String get() = resource.getQuantityString(..., coinCount) State > Orthogonality > Remove with getter/function

Slide 271

Slide 271 text

Replace by using getter/function 2/2 Create coinText value by coinCount var coinCount: Int val coinText: String get() = resource.getQuantityString(..., coinCount) The modiﬁable property is only coinCount State > Orthogonality > Remove with getter/function

Slide 272

Slide 272 text

How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality

Slide 273

Slide 273 text

Encapsulate by algebraic data type 1/2 Example: var queryResultText: String? var queryErrorCode: Int? One of queryResultText or queryErrorCode is non null exclusively State > Orthogonality > Encapsulate with an algebraic class

Slide 274

Slide 274 text

Encapsulate by algebraic data type 1/2 Example: var queryResultText: String? var queryErrorCode: Int? One of queryResultText or queryErrorCode is non null exclusively A variable cannot be computable from the other State > Orthogonality > Encapsulate with an algebraic class

Slide 275

Slide 275 text

Encapsulate by algebraic data type 1/2 Example: var queryResultText: String? var queryErrorCode: Int? One of queryResultText or queryErrorCode is non null exclusively A variable cannot be computable from the other Use algebraic data type State > Orthogonality > Encapsulate with an algebraic class

Slide 276

Slide 276 text

Algebraic data type Data type to represent direct sum State > Orthogonality > Encapsulate with an algebraic class

Slide 277

Slide 277 text

Algebraic data type Data type to represent direct sum Example of binary tree: Node = Branch(Node, Node) | Leaf(Int) | Nil State > Orthogonality > Encapsulate with an algebraic class

Slide 278

Slide 278 text

Algebraic data type Data type to represent direct sum Example of binary tree: Node = Branch(Node, Node) | Leaf(Int) | Nil Realized by: Tagged union, variant, sealed class, associated value ... State > Orthogonality > Encapsulate with an algebraic class

Slide 279

Slide 279 text

Encapsulate by algebraic data type 2/2 sealed class QueryResponse { class Result(val resultText: String): QueryResponse() class Error(val errorCode: Int): QueryResponse() } State > Orthogonality > Encapsulate with an algebraic class

Slide 280

Slide 280 text

Encapsulate by algebraic data type 2/2 sealed class QueryResponse { class Result(val resultText: String): QueryResponse() class Error(val errorCode: Int): QueryResponse() } QueryResponse has resultText or errorCode exclusively State > Orthogonality > Encapsulate with an algebraic class

Slide 281

Slide 281 text

How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality > Encapsulate with an algebraic class

Slide 282

Slide 282 text

How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type - Emulate algebraic data type - Use an enum for a special case State > Orthogonality > Encapsulate with an algebraic class

Slide 283

Slide 283 text

Slide 284

Slide 284 text

Emulate algebraic data type 1/2 Some language does not have algebraic data type (e.g., Java) State > Orthogonality > Encapsulate with an algebraic class

Slide 285

Slide 285 text

Emulate algebraic data type 1/2 Some language does not have algebraic data type (e.g., Java) Emulate algebraic data type with a small class State > Orthogonality > Encapsulate with an algebraic class

Slide 286

Slide 286 text

Emulate algebraic data type 2/2 class QueryResponse { @Nullable private final String resultText; @Nullable private final Integer errorCode; State > Orthogonality > Encapsulate with an algebraic class

Slide 287

Slide 287 text

Slide 288

Slide 288 text

Emulate algebraic data type 2/2 class QueryResponse { @Nullable private final String resultText; @Nullable private final Integer errorCode; private QueryResponse(...) { ... } @NonNull static QueryResponse asResult(@NonNull String ...) { ... } @NonNull static QueryResponse asError(int errorCode) { ... } State > Orthogonality > Encapsulate with an algebraic class

Slide 289

Slide 289 text

Slide 290

Slide 290 text

Use an enum for a special case An enum may be enough to remove a non-orthogonal relationship State > Orthogonality > Encapsulate with an algebraic class

Slide 291

Slide 291 text

Use an enum for a special case An enum may be enough to remove a non-orthogonal relationship // isResultViewShown && isErrorViewShown can't happen var isResultViewShown: Boolean var isErrorViewShown: Boolean State > Orthogonality > Encapsulate with an algebraic class

Slide 292

Slide 292 text

Use an enum for a special case An enum may be enough to remove a non-orthogonal relationship // isResultViewShown && isErrorViewShown can't happen var isResultViewShown: Boolean var isErrorViewShown: Boolean An enum can remove (true, true) case enum class VisibleViewType { RESULT_VIEW, ERROR_VIEW, NOTHING } State > Orthogonality > Encapsulate with an algebraic class

Slide 293

Slide 293 text

"Orthogonal" relationship: Summary State > Orthogonality > Summary

Slide 294

Slide 294 text

"Orthogonal" relationship: Summary Orthogonal relationship - Updatable independently - Good to remove invalid state State > Orthogonality > Summary

Slide 295

Slide 295 text

"Orthogonal" relationship: Summary Orthogonal relationship - Updatable independently - Good to remove invalid state To remove non-orthogonal relationships, use - property getter/function - algebraic data type, enum, small class State > Orthogonality > Summary

Slide 296

Slide 296 text

Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > State design strategy

Slide 297

Slide 297 text

Types of object state transitions State > State design strategy

Slide 298

Slide 298 text

Types of object state transitions - Immutable: e.g., constant value - Idempotent: e.g., closable, lazy - Acyclic (except for self): e.g., resource stream - Cyclic: e.g., reusable State > State design strategy

Slide 299

Slide 299 text

Slide 300

Slide 300 text

Immutable object All properties won't be changed State > State design strategy > Immutable

Slide 301

Slide 301 text

Immutable object All properties won't be changed Immutable type example: class Immutable(val value: Int) class AnotherImmutable(val immutable: Immutable) State > State design strategy > Immutable

Slide 302

Slide 302 text

Immutable object All properties won't be changed Immutable type example: class Immutable(val value: Int) class AnotherImmutable(val immutable: Immutable) Mutable type example: class Mutable(var value: Int) class AnotherMutable(var immutable: Immutable) class YetAnotherMutable(val mutable: Mutable) State > State design strategy > Immutable

Slide 303

Slide 303 text

Immutable properties Make properties immutable if possible State > State design strategy > Immutable

Slide 304

Slide 304 text

Immutable properties Make properties immutable if possible class ItemListCategoryData( // Use val if no need to update var itemCategoryName: String, State > State design strategy > Immutable

Slide 305

Slide 305 text

Immutable properties Make properties immutable if possible class ItemListCategoryData( // Use val if no need to update var itemCategoryName: String, // Even if we need to update, // val of MutableList or var of List is enough var itemModelList: MutableList ) State > State design strategy > Immutable

Slide 306

Slide 306 text

Slide 307

Slide 307 text

Idempotency The result of multiple operations is the same as single operation State > State design strategy > Idempotent

Slide 308

Slide 308 text

Idempotency The result of multiple operations is the same as single operation val closable = Closable() // "OPEN" state closable.close() // "CLOSED" state closable.close() // Valid. Keep "CLOSED" state State > State design strategy > Idempotent

Slide 309

Slide 309 text

Slide 310

Slide 310 text

State graph of idempotent object There are two states and one direction path ┌─────┐ ▼ │ ┌──────┐ ┌──────┐ │ │ Open │──────▶│Closed│──┘ └──────┘ └──────┘ State > State design strategy > Idempotent

Slide 311

Slide 311 text

Why idempotency is good Encapsulate internal state to call a function State > State design strategy > Idempotent

Slide 312

Slide 312 text

Why idempotency is good Encapsulate internal state to call a function // We may forget to check `isClosed` if (!nonIdempotentClosable.isClosed()) { nonIdempotentClosable.close() } State > State design strategy > Idempotent

Slide 313

Slide 313 text

Slide 314

Slide 314 text

Non-idempotent case It's NOT idempotent if the ﬁrst state can remain after the operation State > State design strategy > Idempotent

Slide 315

Slide 315 text

Non-idempotent case It's NOT idempotent if the ﬁrst state can remain after the operation fun get(): Result? { if (latestResult == null) { latestResult = queryToServer() // May return null } return latestResult State > State design strategy > Idempotent

Slide 316

Slide 316 text

Slide 317

Slide 317 text

Non-idempotent state transition graph The ﬁrst state also has a cycle to itself ┌─────┐ ┌─────┐ ▼ │ ▼ │ ┌──────┐ │ ┌──────┐ │ │ Null │──┴───▶│Result│──┘ └──────┘ └──────┘ State > State design strategy > Idempotent

Slide 318

Slide 318 text

Non-idempotent state transition graph The ﬁrst state also has a cycle to itself ┌─────┐ ┌─────┐ ▼ │ ▼ │ ┌──────┐ │ ┌──────┐ │ │ Null │──┴───▶│Result│──┘ └──────┘ └──────┘ The side-effect should not be hidden State > State design strategy > Idempotent

Slide 319

Slide 319 text

Slide 320

Slide 320 text

Acyclic state Instance only used for a speciﬁc argument - Another instance is created for another argument - The performance is not good sometimes State > State design strategy > Acyclic

Slide 321

Slide 321 text

Acyclic state Instance only used for a speciﬁc argument - Another instance is created for another argument - The performance is not good sometimes // This class instance works only a given `videoPath`. class VideoPlayer(videoPath: String) { enum class State { LOADING, PLAYING, FINISHED, ERROR } ... State > State design strategy > Acyclic

Slide 322

Slide 322 text

State graph of acyclic state Partial order like state transition (if we ignore loop for itself) ┌────────┐ ┌────────┐ ┌────────┐ │Loading │─▶│Playing │──▶│Finished│ └────────┘ └────────┘ └────────┘ │ │ ┌────────┐ └───────────┴──────▶│ Error │ └────────┘ State > State design strategy > Acyclic

Slide 323

Slide 323 text

Why acyclic state is good 1/2 Let's compare with a type with cyclic states State > State design strategy > Acyclic

Slide 324

Slide 324 text

Why acyclic state is good 1/2 Let's compare with a type with cyclic states Cyclic type example: class VideoPlayer { fun play(videoPath: String) { ... } State > State design strategy > Acyclic

Slide 325

Slide 325 text

Cyclic state transition graph Loopback to "Loading" with a new video path ┌───────────────────────────────┐ ▼ │ ┌────────┐ ┌────────┐ ┌────────┐ │ │Loading │─▶│Playing │──▶│Finished│──┤ └────────┘ └────────┘ └────────┘ │ │ │ ┌────────┐ │ └───────────┴──────▶│ Error │──┘ └────────┘ State > State design strategy > Acyclic

Slide 326

Slide 326 text

Why acyclic state is good 2/2 Need to check the current loop argument if there's a cycle State > State design strategy > Acyclic

Slide 327

Slide 327 text

Why acyclic state is good 2/2 Need to check the current loop argument if there's a cycle // // Abort playing video after 1000ms launch(...) { delay(1000) videoPlayer.finish() State > State design strategy > Acyclic

Slide 328

Slide 328 text

Why acyclic state is good 2/2 Need to check the current loop argument if there's a cycle val videoPath = videoPlayer.videoPath // Abort playing video after 1000ms launch(...) { delay(1000) // Need to check if a new video is not loaded if (videoPath == videoPlayer.videoPath) { videoPlayer.finish() State > State design strategy > Acyclic

Slide 329

Slide 329 text

Slide 330

Slide 330 text

Acyclic state VS reality A cycle is often required to make a model simply - May be overly complex to remove all cycles - Example of VideoPlayer: PLAYING <-> PAUSED State > State design strategy > Acyclic

Slide 331

Slide 331 text

Acyclic state VS reality A cycle is often required to make a model simply - May be overly complex to remove all cycles - Example of VideoPlayer: PLAYING <-> PAUSED Make the cycle as small as possible State > State design strategy > Acyclic

Slide 332

Slide 332 text

State cycle encapsulation Put a cycle in an enclosing state = Don't create large cycle ┌────────────────────┐ ┌────────┐ │┌───────┐─▶┌───────┐│ ┌────────┐ │Loading │──▶││Playing│ │Paused ││──▶│Finished│ └────────┘ │└───────┘◀─└───────┘│ └────────┘ │ └────────────────────┘ │ │ ┌────────┐ └──────────────────┴────────────▶│ Error │ └────────┘ State > State design strategy > Acyclic

Slide 333

Slide 333 text

Slide 334

Slide 334 text

State design strategy: Summary - Make properties immutable - Idempotency is also good - Remove/minimize state cycle State > State design strategy > Summary

Slide 335

Slide 335 text

Summary Minimize state for readability and robustness - Don't make it an objective State > Summary

Slide 336

Slide 336 text

Summary Minimize state for readability and robustness - Don't make it an objective Remove/encapsulate non-orthogonal relationships - Use a property getter/function, an algebraic data type, or enum State > Summary

Slide 337

Slide 337 text

Slide 338

Slide 338 text

abstract fun codeReadabilitySession5() fun Procedure()

Slide 339

Slide 339 text

Slide 340

Slide 340 text

What "procedure" is - main/subroutine - function - method - computed property - property getter/setter - constructor/initialization block - ... Procedure > Introduction

Slide 341

Slide 341 text

What a readable procedure is Procedure > Introduction

Slide 342

Slide 342 text

What a readable procedure is Predictable - Consistent with the name - Easy to write documentation - Few error cases or limitations Procedure > Introduction

Slide 343

Slide 343 text

Topics Make procedure responsibility/ﬂow clear Procedure > Introduction

Slide 344

Slide 344 text

Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make ﬂow clear - Perform deﬁnition-based programming - Focus on normal cases - Split by object, not condition Procedure > Introduction

Slide 345

Slide 345 text

Slide 346

Slide 346 text

Split procedure if it's hard to summarize Try to summarize what the procedure does Procedure > Responsibility > Check with a short summary

Slide 347

Slide 347 text

Procedure summary: Example 1/2 messageView.text = messageData.contentText senderNameView.text = messageData.senderName timestampView.text = messageData.sentTimeText Procedure > Responsibility > Check with a short summary

Slide 348

Slide 348 text

Procedure summary: Example 1/2 messageView.text = messageData.contentText senderNameView.text = messageData.senderName timestampView.text = messageData.sentTimeText We can summarize this as: "Bind/update message layout with a new message data" Procedure > Responsibility > Check with a short summary

Slide 349

Slide 349 text

Procedure summary: Example 2/2 messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } Procedure > Responsibility > Check with a short summary

Slide 350

Slide 350 text

Procedure summary: Example 2/2 messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } How to summarize this Procedure > Responsibility > Check with a short summary

Slide 351

Slide 351 text

Procedure summary: Example 2/2 messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } How to summarize this "Bind a new message and save it" "Perform actions on a new message received" Procedure > Responsibility > Check with a short summary

Slide 352

Slide 352 text

Procedure responsibility and summary Hard to summarize = Typical bad signal - Split the procedure into sub-procedures Procedure > Responsibility > Check with a short summary

Slide 353

Slide 353 text

Procedure responsibility and summary Hard to summarize = Typical bad signal - Split the procedure into sub-procedures fun bindMessageViewData(messageData: MessageData) { ... fun saveMessageDataToDatabase(messageData: MessageData) { ... Procedure > Responsibility > Check with a short summary

Slide 354

Slide 354 text

Slide 355

Slide 355 text

Command-query separation6 Asking a question should not change the answer 7 7 Eiffel: a language for software engineering, Meyer Bertrand, 2012 6 https://martinfowler.com/bliki/CommandQuerySeparation.html Procedure > Responsibility > Return value and side-effect

Slide 356

Slide 356 text

Command-query separation6 Asking a question should not change the answer 7 Split procedure by command and query - Command: Procedure to modify receiver or parameters - Query: Procedure to return without any modiﬁcation 7 Eiffel: a language for software engineering, Meyer Bertrand, 2012 6 https://martinfowler.com/bliki/CommandQuerySeparation.html Procedure > Responsibility > Return value and side-effect

Slide 357

Slide 357 text

Command-query separation: Example 1/2 class IntList(vararg elements: Int) { infix fun append(others: IntList): IntList = ... } Procedure > Responsibility > Return value and side-effect

Slide 358

Slide 358 text

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 Procedure > Responsibility > Return value and side-effect

Slide 359

Slide 359 text

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? Procedure > Responsibility > Return value and side-effect

Slide 360

Slide 360 text

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} Procedure > Responsibility > Return value and side-effect

Slide 361

Slide 361 text

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} Result with bad code: a={1, 2, 3, 4}, b={3, 4}, c={1, 2, 3, 4} Procedure > Responsibility > Return value and side-effect

Slide 362

Slide 362 text

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} Result with bad code: a={1, 2, 3, 4}, b={3, 4}, c={1, 2, 3, 4} Function append must not modify a or b if it returns the result Procedure > Responsibility > Return value and side-effect

Slide 363

Slide 363 text

Command-query separation: Drawbacks 1/3 Command-query separation is not an objective - May cause strong coupling (will explain at the next session) - May make an unnecessary state Procedure > Responsibility > Return value and side-effect

Slide 364

Slide 364 text

Command-query separation: Drawbacks 2/3 Bad code example: class UserDataStore { // Command fun saveUserData(userData: UserData) = ... } Procedure > Responsibility > Return value and side-effect

Slide 365

Slide 365 text

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() = ... } Procedure > Responsibility > Return value and side-effect

Slide 366

Slide 366 text

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() = ... } State latestOperationResult may become a cause of a bug Procedure > Responsibility > Return value and side-effect

Slide 367

Slide 367 text

Command-query separation: Drawbacks 2/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 = ... } Procedure > Responsibility > Return value and side-effect

Slide 368

Slide 368 text

Command-query separation: Drawbacks 2/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 Procedure > Responsibility > Return value and side-effect

Slide 369

Slide 369 text

Responsibility on return value and side-effect Don't modify when returning a "main" result Procedure > Responsibility > Return value and side-effect

Slide 370

Slide 370 text

Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modiﬁcation Procedure > Responsibility > Return value and side-effect

Slide 371

Slide 371 text

Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modiﬁcation - Main result: conversion/calculation result, a new instance ... - Sub result: error type, metadata of modiﬁcation (stored size) ... (Documentation is mandatory) Procedure > Responsibility > Return value and side-effect

Slide 372

Slide 372 text

Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modiﬁcation - Main result: conversion/calculation result, a new instance ... - Sub result: error type, metadata of modiﬁcation (stored size) ... (Documentation is mandatory) Acceptable if the interface is a de facto standard e.g., fun Queue.poll(): T Procedure > Responsibility > Return value and side-effect

Slide 373

Slide 373 text

Make responsibility clear: Summary Procedure > Responsibility > Summary

Slide 374

Slide 374 text

Make responsibility clear: Summary Split a procedure if it's hard to summarize - Try to write documentation anyway Procedure > Responsibility > Summary

Slide 375

Slide 375 text

Make responsibility clear: Summary Split a procedure if it's hard to summarize - Try to write documentation anyway Don't modify when returning a "main" result - Fine to return "sub" result with modiﬁcation - Fine if the interface is a de facto standard Procedure > Responsibility > Summary

Slide 376

Slide 376 text

Slide 377

Slide 377 text

How to ﬁnd if procedure ﬂow is clear Procedure > Flow

Slide 378

Slide 378 text

How to ﬁnd if procedure ﬂow is clear Try to write short summary Procedure > Flow

Slide 379

Slide 379 text

How to find if procedure flow is clear Try to write short summary - Hard to write when the flow is unclear Procedure > Flow

Slide 380

Slide 380 text

Slide 381

Slide 381 text

Definition-based programming Prefer to define value/procedure with a name Procedure > Flow > Definition-based programming

Slide 382

Slide 382 text

Definition-based programming Prefer to define value/procedure with a name rather than: - Nest (parameter, procedure, type, control flow) - Anonymous procedure/object literal - Chained call Procedure > Flow > Definition-based programming

Slide 383

Slide 383 text

Deﬁnition-based programming: Bad example Procedure > Flow > Deﬁnition-based programming

Slide 384

Slide 384 text

Deﬁnition-based programming: Bad example fun startColorChangeAnimation(startColorInt: Int, endColorInt: Int) = ColorAnimator(srcColorInt, destinationColorInt) .also { it.addUpdateListener { if (it.colorValue == null) { return@addUpdateListener } // Apply the new color to views } }.start() Procedure > Flow > Deﬁnition-based programming

Slide 385

Slide 385 text

What is wrong with the bad example Hard to make summary of the procedure Procedure > Flow > Deﬁnition-based programming

Slide 386

Slide 386 text

What is wrong with the bad example Hard to make summary of the procedure - Context and scope because of nested lambdas - Which code is called directly, and which asynchronously - Receiver caused by overly long call chain Procedure > Flow > Deﬁnition-based programming

Slide 387

Slide 387 text

Definition-based programming: How to fix 1/3 Extract a lambda/parameter/receiver/call-chain as a named local value or a named private function Procedure > Flow > Definition-based programming

Slide 388

Slide 388 text

Definition-based programming: How to fix 2/3 fun startColorChangeAnimation(startColorInt: Int, endColorInt: Int) = ColorAnimator(srcColorInt, destinationColorInt) .also { it.addUpdateListener { if (it.colorValue == null) { return@addUpdateListener } // Apply the new color to views } }.start() Procedure > Flow > Definition-based programming

Slide 389

Slide 389 text

Slide 390

Slide 390 text

Definition-based programming: How to fix 2/3 fun startColorChangeAnimation(startColorInt: Int, endColorInt: Int) = ColorAnimator(srcColorInt, destinationColorInt) .also { it.addUpdateListener { applyColorToViews(it.colorValue) } }.start() fun applyColorToViews(colorInt: Int?) { if (colorInt == null) { return } // Apply the new color to views } Procedure > Flow > Definition-based programming

Slide 391

Slide 391 text

Definition-based programming: How to fix 3/3 fun startColorChangeAnimation(startColorInt: Int, endColorInt: Int) = ColorAnimator(srcColorInt, destinationColorInt) .also { it.addUpdateListener { applyColorToViews(it.colorValue) } }.start() fun applyColorToViews(colorInt: Int?) { if (colorInt == null) { return } // Apply the new color to views } Procedure > Flow > Definition-based programming

Slide 392

Slide 392 text

Definition-based programming: How to fix 3/3 fun startColorChangeAnimation(startColorInt: Int, endColorInt: Int) { val animator = ColorAnimator(srcColorInt, destinationColorInt) animator.addUpdateListener { applyColorToViews(it.colorValue) } animator.start() } fun applyColorToViews(colorInt: Int?) { if (colorInt == null) { return } // Apply the new color to views } Procedure > Flow > Definition-based programming

Slide 393

Slide 393 text

Slide 394

Slide 394 text

Deﬁnition-based programming: Pitfall Extracting may make the code less readable Procedure > Flow > Deﬁnition-based programming

Slide 395

Slide 395 text

Deﬁnition-based programming: Pitfall Extracting may make the code less readable - Unnecessary state - Unnecessary strong coupling (will explain at the next session) Procedure > Flow > Deﬁnition-based programming

Slide 396

Slide 396 text

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 Procedure > Flow > Deﬁnition-based programming

Slide 397

Slide 397 text

Pitfalls of extraction: Bad example var userNameTextView: View? = null var profileImageView: View? = null init { initializeUserNameTextView() initializeProfileImageView() } private fun initializeUserNameTextView() { userNameTextView = ... private fun initializeProfileImageView() { profileImageView = ... Procedure > Flow > Deﬁnition-based programming

Slide 398

Slide 398 text

Pitfalls of extraction: What is wrong - Unnecessary mutability and nullability - Unsafe to call initialize... methods multiple times - Function name initialize... is ambiguous Procedure > Flow > Deﬁnition-based programming

Slide 399

Slide 399 text

Slide 400

Slide 400 text

Pitfalls of extraction: Good example Extract view creation and initialization procedure Procedure > Flow > Deﬁnition-based programming

Slide 401

Slide 401 text

Pitfalls of extraction: Good example Extract view creation and initialization procedure val userNameTextView: View = createUserNameTextView() val profileImageView: View = createProfileImageView() private fun createUserNameTextView(): TextView = ... private fun createProfileImageView(): ImageView = ... Procedure > Flow > Deﬁnition-based programming

Slide 402

Slide 402 text

Slide 403

Slide 403 text

Normal cases and error cases Normal case: Achieves the main purpose of the procedure Error case: Quits the procedure without achieving the purpose Procedure > Flow > Focus on normal cases

Slide 404

Slide 404 text

Normal cases and error cases: Example String.toIntOrNull() Procedure > Flow > Focus on normal cases

Slide 405

Slide 405 text

Normal cases and error cases: Example String.toIntOrNull() Normal case: Returns an integer value e.g, "-1234", "0", "+001", "-2147483648" Procedure > Flow > Focus on normal cases

Slide 406

Slide 406 text

Normal cases and error cases: Example String.toIntOrNull() Normal case: Returns an integer value e.g, "-1234", "0", "+001", "-2147483648" Error case: Returns null e.g., "--0", "text", "", "2147483648", "1.0" Procedure > Flow > Focus on normal cases

Slide 407

Slide 407 text

Procedure ﬂow and normal/error cases Procedure > Flow > Focus on normal cases

Slide 408

Slide 408 text

Procedure ﬂow and normal/error cases Filter error cases to focus on the normal case Procedure > Flow > Focus on normal cases

Slide 409

Slide 409 text

Procedure ﬂow and normal/error cases Filter error cases to focus on the normal case Apply early return Procedure > Flow > Focus on normal cases

Slide 410

Slide 410 text

Early return: Bad example Procedure > Flow > Focus on normal cases

Slide 411

Slide 411 text

Early return: Bad example if (isNetworkAvailable()) { val queryResult = queryToServer() if (queryResult.isValid) { // Do something with queryResult... } else { showInvalidResponseDialog() } } else { showNetworkUnavailableDialog() } Procedure > Flow > Focus on normal cases

Slide 412

Slide 412 text

Early return: What is wrong Unclear Procedure > Flow > Focus on normal cases

Slide 413

Slide 413 text

Early return: What is wrong Unclear - What the main case logic is - The relation between error condition and handling logic Procedure > Flow > Focus on normal cases

Slide 414

Slide 414 text

Early return: Good example Procedure > Flow > Focus on normal cases

Slide 415

Slide 415 text

Early return: Good example if (!isNetworkAvailable()) { showNetworkUnavailableDialog() return } val queryResult = queryToServer() if (!queryResult.isValid) { showInvalidResponseDialog() return } // Do something with queryResult... Procedure > Flow > Focus on normal cases

Slide 416

Slide 416 text

Benefits of early return Easy to find - The main flow and purpose of a procedure - Relation between error condition and handling logic Procedure > Flow > Focus on normal cases

Slide 417

Slide 417 text

Slide 418

Slide 418 text

Two axes to break down procedure Multiple conditions and target objects Example: - Conditions: Two account types (premium, free) - Objects: Two views (background, icon) Procedure > Flow > Split by object

Slide 419

Slide 419 text

Two axes to break down procedure Multiple conditions and target objects Condition ┃ Premium account │ Free account Object ┃ │ ━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━ Background color ┃ Yellow │ Gray ───────────────────╂─────────────────┼──────────────── Account Icon ┃ [Premium] │ [Free] Procedure > Flow > Split by object

Slide 420

Slide 420 text

Slide 421

Slide 421 text

Slide 422

Slide 422 text

Bad example of "split by condition ﬁrst" 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) } Procedure > Flow > Split by object

Slide 423

Slide 423 text

Split by object: What is wrong - Can't summarize what the function does /** * Updates views depending whether the account type is free or premium. */ Procedure > Flow > Split by object

Slide 424

Slide 424 text

Split by object: What is wrong - Can't summarize what the function does /** * Updates views depending whether the account type is free or premium. */ - May cause bug because of no guarantee of completeness Procedure > Flow > Split by object

Slide 425

Slide 425 text

Slide 426

Slide 426 text

Split by object: Good example 1/2 fun bindViewData(accountType: AccountType) { updateBackgroundViewColor(accountType) updateAccountTypeImage(accountType) } Procedure > Flow > Split by object

Slide 427

Slide 427 text

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 } } Procedure > Flow > Split by object

Slide 428

Slide 428 text

Split by object: Good example - Easy to write a short summary /** * Updates background color and icon according to a given account type. */ Procedure > Flow > Split by object

Slide 429

Slide 429 text

Split by object: Good example - Easy to write a short summary /** * Updates background color and icon according to a given account type. */ - Ensures that all the combinations are covered Procedure > Flow > Split by object

Slide 430

Slide 430 text

Slide 431

Slide 431 text

Split by object: More improvement 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 } Procedure > Flow > Split by object

Slide 432

Slide 432 text

Early return VS split by object Doesn't "early return" represent "split by condition"? Procedure > Flow > Split by object

Slide 433

Slide 433 text

Early return VS split by object Doesn't "early return" represent "split by condition"? Strategy 1: - Handle an error case as if it's a normal case Procedure > Flow > Split by object

Slide 434

Slide 434 text

Early return VS split by object Doesn't "early return" represent "split by condition"? Strategy 1: - Handle an error case as if it's a normal case Strategy 2: - Apply early return to error cases - Apply "split by object" Procedure > Flow > Split by object

Slide 435

Slide 435 text

Procedure ﬂow: Summary Procedure > Flow > Summary

Slide 436

Slide 436 text

Procedure ﬂow: Summary Perform deﬁnition-based programming: Extract nests/chains to a named value/function Procedure > Flow > Summary

Slide 437

Slide 437 text

Procedure ﬂow: Summary Perform deﬁnition-based programming: Extract nests/chains to a named value/function Focus on normal cases: Apply early return Procedure > Flow > Summary

Slide 438

Slide 438 text

Procedure ﬂow: Summary Perform deﬁnition-based programming: Extract nests/chains to a named value/function Focus on normal cases: Apply early return Split by object, not condition: Make a function or block for each target object Procedure > Flow > Summary

Slide 439

Slide 439 text

Summary Check whether it's easy to write a short summary Procedure > Summary

Slide 440

Slide 440 text

Summary Check whether it's easy to write a short summary Procedure responsibility: - Split if required - Don't modify when returning a main result Procedure > Summary

Slide 441

Slide 441 text

Summary Check whether it's easy to write a short summary Procedure responsibility: - Split if required - Don't modify when returning a main result Procedure ﬂow: - Apply deﬁnition based programing, early return, split by object Procedure > Summary

Slide 442

Slide 442 text

Code→readability→session 6 Dependency ‐

Slide 443

Slide 443 text

Slide 444

Slide 444 text

What "dependency" is Example: Type "X" depends on type "Y" - Type X has an instance of Y as a property - A method of X takes Y as a parameter or returns Y - X calls a method of Y - X inherits Y Dependency

Slide 445

Slide 445 text

What "dependency" is Example: Type "X" depends on type "Y" - Type X has an instance of Y as a property - A method of X takes Y as a parameter or returns Y - X calls a method of Y - X inherits Y "X" depends on "Y" if word "Y" appears in "X", roughly speaking Dependency

Slide 446

Slide 446 text

What "dependency" is Is the word "dependency" only for class? Dependency

Slide 447

Slide 447 text

What "dependency" is Is the word "dependency" only for class? No - Procedure, module, instance ... Dependency

Slide 448

Slide 448 text

Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency

Slide 449

Slide 449 text

Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Coupling

Slide 450

Slide 450 text

Coupling A property to represent how strong the dependency is ▲ Strong │ │ │ │ │ │ ▼ Weak Dependency > Coupling

Slide 451

Slide 451 text

Coupling A property to represent how strong the dependency is ▲ Strong │ Content coupling │ Common coupling │ Control coupling │ Stamp coupling │ Data coupling │ Message coupling ▼ Weak (Exception: External coupling) Dependency > Coupling

Slide 452

Slide 452 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling

Slide 453

Slide 453 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Content coupling

Slide 454

Slide 454 text

Content coupling Relies on the internal workings Dependency > Coupling > Content coupling

Slide 455

Slide 455 text

Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling

Slide 456

Slide 456 text

Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling

Slide 457

Slide 457 text

Allows illegal usage: Bad example class Caller { fun callCalculator() { calculator.calculate() ... Dependency > Coupling > Content coupling

Slide 458

Slide 458 text

Allows illegal usage: Bad example class Caller { fun callCalculator() { calculator.parameter = 42 calculator.calculate() val result = calculator.result ... Dependency > Coupling > Content coupling

Slide 459

Slide 459 text

Allows illegal usage: Bad example class Caller { fun callCalculator() { calculator.parameter = 42 calculator.prepare() calculator.calculate() calculator.tearDown() val result = calculator.result ... Dependency > Coupling > Content coupling

Slide 460

Slide 460 text

Allows illegal usage: Bad points Dependency > Coupling > Content coupling

Slide 461

Slide 461 text

Allows illegal usage: Bad points calculator has the following two requirements: - Call prepare()/tearDown() before/after calling calculate() - Pass/get a value by parameter/result property. Dependency > Coupling > Content coupling

Slide 462

Slide 462 text

Slide 463

Slide 463 text

Allows illegal usage: How to ﬁx - Encapsulate stateful call sequence - Pass/receive data as arguments or a return value Dependency > Coupling > Content coupling

Slide 464

Slide 464 text

Allows illegal usage: Fixed code example class Caller { fun callCalculator() { val result = calculator.calculate(42) ... Dependency > Coupling > Content coupling

Slide 465

Slide 465 text

Allows illegal usage: Fixed code example class Caller { fun callCalculator() { val result = calculator.calculate(42) ... class Calculator { fun calculate(type: Int): Int { prepare() ... tearDown() return ... Dependency > Coupling > Content coupling

Slide 466

Slide 466 text

Allows illegal usage: What to check Dependency > Coupling > Content coupling

Slide 467

Slide 467 text

Allows illegal usage: What to check Don't apply design concept/principal too much - e.g., command/query separation Dependency > Coupling > Content coupling

Slide 468

Slide 468 text

Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling

Slide 469

Slide 469 text

Shares mutable state: Bad example class UserListPresenter(val userList: List) { fun refreshViews() { /* ... */ } Dependency > Coupling > Content coupling

Slide 470

Slide 470 text

Shares mutable state: Bad example class UserListPresenter(val userList: List) { fun refreshViews() { /* ... */ } class Caller { val userList: List = mutableListOf() val presenter = UserListPresenter(userList) ... Dependency > Coupling > Content coupling

Slide 471

Slide 471 text

Slide 472

Slide 472 text

Slide 473

Slide 473 text

Shares mutable state: Bad points Behavior of UserListPresenter depends on external mutable value - Causes unexpected state update by other reference holders - No limitation of modifying the list Easy to break the state and hard to ﬁnd the root cause - A reference to the presenter is not required to update it Dependency > Coupling > Content coupling

Slide 474

Slide 474 text

Shares mutable state: How to ﬁx - Make values immutable, copy arguments, or apply copy-on-write - Limit interfaces to update the state Dependency > Coupling > Content coupling

Slide 475

Slide 475 text

Shares mutable state: Fixed code example class UserListPresenter { val userList: MutableList = mutableListOf() Dependency > Coupling > Content coupling

Slide 476

Slide 476 text

Shares mutable state: Fixed code example class UserListPresenter { val userList: MutableList = mutableListOf() fun addUsers(newUsers: List) { userList += newUsers // Update views with `userList` } Dependency > Coupling > Content coupling

Slide 477

Slide 477 text

Slide 478

Slide 478 text

Shares mutable state: What to check Dependency > Coupling > Content coupling

Slide 479

Slide 479 text

Shares mutable state: What to check Make a mutable value ownership clear Dependency > Coupling > Content coupling

Slide 480

Slide 480 text

Shares mutable state: What to check Make a mutable value ownership clear Options: - Modify only by owner's interface - Create manager class of mutable data Dependency > Coupling > Content coupling

Slide 481

Slide 481 text

Content coupling: Summary Don't rely on internal working - Remove illegal usage - Make mutable data ownership clear Dependency > Coupling > Content coupling

Slide 482

Slide 482 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Common coupling

Slide 483

Slide 483 text

Common coupling Shares (mutable) global data Dependency > Coupling > Common coupling

Slide 484

Slide 484 text

Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use ﬁles / databases / shared resources) Dependency > Coupling > Common coupling

Slide 485

Slide 485 text

Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use ﬁles / databases / shared resources) Dependency > Coupling > Common coupling

Slide 486

Slide 486 text

Pass data by global variables: Bad example var parameter: Int? = null var result: Data? = null Dependency > Coupling > Common coupling

Slide 487

Slide 487 text

Pass data by global variables: Bad example var parameter: Int? = null var result: Data? = null class Calculator { fun calculate() { result = parameter + ... Dependency > Coupling > Common coupling

Slide 488

Slide 488 text

Pass data by global variables: Bad example var parameter: Int? = null var result: Data? = null class Calculator { fun calculate() { result = parameter + ... fun callCalculator() { parameter = 42 calculator.calculate() Dependency > Coupling > Common coupling

Slide 489

Slide 489 text

Pass data by global variables: Bad points No information/limitation of call relationships - Breaks other simultaneous calls - Tends to cause illegal state Dependency > Coupling > Common coupling

Slide 490

Slide 490 text

Pass data by global variables: How to ﬁx Pass as a function parameter or return value Dependency > Coupling > Common coupling

Slide 491

Slide 491 text

Pass data by global variables: Fixed code class Calculator { fun calculate(parameter: Int): Int = parameter + ... Dependency > Coupling > Common coupling

Slide 492

Slide 492 text

Pass data by global variables: Fixed code class Calculator { fun calculate(parameter: Int): Int = parameter + ... fun callCalculator() { val result = calculator.calculate(42) Dependency > Coupling > Common coupling

Slide 493

Slide 493 text

Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use ﬁles / databases / shared resources) Dependency > Coupling > Common coupling

Slide 494

Slide 494 text

Depend on a mutable singleton: Bad example val USER_DATA_REPOSITORY = UserDataRepository() Dependency > Coupling > Common coupling

Slide 495

Slide 495 text

Depend on a mutable singleton: Bad example val USER_DATA_REPOSITORY = UserDataRepository() class UserListUseCase { suspend fun invoke(): List = withContext(...) { val result = USER_DATA_REPOSITORY.query(...) ... Dependency > Coupling > Common coupling

Slide 496

Slide 496 text

Mutable global variables: Bad points Singleton is not managed - Unmanaged singleton lifecycle, scope, and references - Not robust for speciﬁcation change - Bad testability Dependency > Coupling > Common coupling

Slide 497

Slide 497 text

Mutable global variables: How to ﬁx Pass an instance as a constructor parameter (= Dependency injection) The constructor caller can manage the instance Dependency > Coupling > Common coupling

Slide 498

Slide 498 text

Mutable global variables: Fixed code class UserListUseCase( val userDataRepository: UserDataRepository ) { suspend fun invoke(): List = withContext(...) { val result = userDataRepository.query(...) ... Dependency > Coupling > Common coupling

Slide 499

Slide 499 text

Common coupling: Note Apply the same discussion to a smaller scope - module, package, type ... Example: Use a return value instead of instance ﬁeld Dependency > Coupling > Common coupling

Slide 500

Slide 500 text

Common coupling in a class: Bad example class Klass { var result: Result? = null fun firstFunction() { secondFunction() /* use `result` */ ... fun secondFunction() { result = /* calculate result */ Dependency > Coupling > Common coupling

Slide 501

Slide 501 text

Common coupling in a class: Fixed code class Klass { fun firstFunction() { val result = secondFunction() /* use `result` */ ... fun secondFunction(): Int = /* calculate a return value */ Dependency > Coupling > Common coupling

Slide 502

Slide 502 text

Common coupling: Summary No more singletons nor global mutable values - Unmanageable, not robust, hard to test ... Dependency > Coupling > Common coupling

Slide 503

Slide 503 text

Common coupling: Summary No more singletons nor global mutable values - Unmanageable, not robust, hard to test ... To remove common coupling: - Pass value as arguments or a return value - Use dependency injection Dependency > Coupling > Common coupling

Slide 504

Slide 504 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Control coupling

Slide 505

Slide 505 text

Control coupling Switches logic by passing ﬂag "what to do" - Has large conditional branch covering procedure - Each branch body is unrelated to the other branches Dependency > Coupling > Control coupling

Slide 506

Slide 506 text

Control coupling example: Boolean ﬂag fun updateView(isError: Boolean) { if (isError) { resultView.isVisible = true errorView.isVisible = false iconView.image = CROSS_MARK_IMAGE } else { resultView.isVisible = false errorView.isVisible = true iconView.image = CHECK_MARK_IMAGE } } Dependency > Coupling > Control coupling

Slide 507

Slide 507 text

Control coupling example: "To do" type fun updateUserView(dataType: DataType) = when(dataType) { UserName -> { val userName = getUserName() userNameView.text = userName } Birthday -> { val birthdayMillis = ... birthDayView.text = format(...) } ProfileImage -> ... } Dependency > Coupling > Control coupling

Slide 508

Slide 508 text

Control coupling: Bad points Unreadable - Hard to summarize what it does - Difﬁcult to name and comment Dependency > Coupling > Control coupling

Slide 509

Slide 509 text

Control coupling: Bad points Unreadable - Hard to summarize what it does - Difﬁcult to name and comment Not robust - Requires the similar conditional branch to the callers - Fragile against condition update Dependency > Coupling > Control coupling

Slide 510

Slide 510 text

Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling

Slide 511

Slide 511 text

Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling

Slide 512

Slide 512 text

Remove conditional branch Split procedures for each argument - Remove parameter from each new procedure Dependency > Coupling > Control coupling

Slide 513

Slide 513 text

Remove conditional branch Split procedures for each argument - Remove parameter from each new procedure Applicable if "what to do" value is different for each caller - If not, causes another content coupling on the caller side Dependency > Coupling > Control coupling

Slide 514

Slide 514 text

Remove conditional branch: Example fun updateUserView(dataType: DataType) = when(dataType) { UserName -> { val userName = getUserName() userNameView.text = userName } Birthday -> { val birthdayMillis = ... birthDayView.text = format(...) } ProfileImage -> ... } Dependency > Coupling > Control coupling

Slide 515

Slide 515 text

Remove conditional branch: Example fun updateUserNameView() { val userName = getUserName() userNameView.text = userName } fun updateBirthdayView() { val birthdayMillis = ... birthDayView.text = format(...) } fun updateProfileImage() { ... Dependency > Coupling > Control coupling

Slide 516

Slide 516 text

Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling

Slide 517

Slide 517 text

Split by object Create code block for each target object, not condition - Break down content coupling into smaller scope Dependency > Coupling > Control coupling

Slide 518

Slide 518 text

Split by object Create code block for each target object, not condition - Break down content coupling into smaller scope Applicable if target objects are common for each condition - doSomethingIf... function may be used Dependency > Coupling > Control coupling

Slide 519

Slide 519 text

Split by object: Example fun updateView(isError: Boolean) { if (isError) { resultView.isVisible = true errorView.isVisible = false iconView.image = CROSS_MARK_IMAGE } else { resultView.isVisible = false errorView.isVisible = true iconView.image = CHECK_MARK_IMAGE } } Dependency > Coupling > Control coupling

Slide 520

Slide 520 text

Split by object: Example fun updateView(isError: Boolean) { resultView.isVisible = !isError errorView.isVisible = isError iconView.image = getIconImage(isError) } fun getIconImage(isError: Boolean): Image = if (!isError) CHECK_MARK_IMAGE else CROSS_MARK_IMAGE Dependency > Coupling > Control coupling

Slide 521

Slide 521 text

Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling

Slide 522

Slide 522 text

Apply strategy pattern Create types to decide "what to do" - Branch with sub-typing, polymorphism, or parameter - Implemented by an enum, an abstract class, or a callback object Dependency > Coupling > Control coupling

Slide 523

Slide 523 text

Apply strategy pattern Create types to decide "what to do" - Branch with sub-typing, polymorphism, or parameter - Implemented by an enum, an abstract class, or a callback object Type selection logic appears on the caller side Dependency > Coupling > Control coupling

Slide 524

Slide 524 text

Apply strategy pattern: Example enum class Binder(val viewId: ViewId) { fun updateView(holder: ViewHolder) = holder.getView(viewId).let(::setContent) abstract fun setContent(view: View) Dependency > Coupling > Control coupling

Slide 525

Slide 525 text

Apply strategy pattern: Example enum class Binder(val viewId: ViewId) { USER_NAME(USER_NAME_VIEW_ID) { override fun setContent(...) = ... } BIRTHDAY(BIRTHDAY_VIEW_ID) { override fun setContent(...) = ... ... fun updateView(holder: ViewHolder) = holder.getView(viewId).let(::setContent) abstract fun setContent(view: View) Dependency > Coupling > Control coupling

Slide 526

Slide 526 text

Control coupling: Summary Don't create large conditional branch covering a whole procedure Dependency > Coupling > Control coupling

Slide 527

Slide 527 text

Control coupling: Summary Don't create large conditional branch covering a whole procedure To lease control coupling: - Remove condition parameters - Split by target instead of condition - Use strategy pattern Dependency > Coupling > Control coupling

Slide 528

Slide 528 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Stamp coupling

Slide 529

Slide 529 text

Stamp coupling Passes a structure though some of the data that are not used Dependency > Coupling > Stamp coupling

Slide 530

Slide 530 text

Stamp coupling Passes a structure though some of the data that are not used Acceptable for some situations: - To use sub-typing, structural typing, or strategy pattern - To simplify parameter types Make parameter type as small as possible Dependency > Coupling > Stamp coupling

Slide 531

Slide 531 text

Stamp coupling: Example updateUserView(userData) fun updateUserView(userData: UserData) { Dependency > Coupling > Stamp coupling

Slide 532

Slide 532 text

Stamp coupling: Example updateUserView(userData) fun updateUserView(userData: UserData) { // Some property of `userData` are used userNameView.text = userData.fullName profileImage.setImageUrl(userData.profileImageUrl) // `userData.mailAddress` and `.phoneNumber` are not used } Dependency > Coupling > Stamp coupling

Slide 533

Slide 533 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Data coupling

Slide 534

Slide 534 text

Data coupling Passes a set of elemental data - All the data must be used fun updateUserView(fullName: String, profileImageUrl: Url) { userNameView.text = fullName profileImage.setImageUrl(profileImageUrl) } updateUserView(userData.fullName, userData.profileImageUrl) Dependency > Coupling > Data coupling

Slide 535

Slide 535 text

Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Message coupling

Slide 536

Slide 536 text

Message coupling Call a method without any data - Hardly happens - May cause content coupling in another layer Dependency > Coupling > Message coupling

Slide 537

Slide 537 text

Message coupling Call a method without any data - Hardly happens - May cause content coupling in another layer fun notifyUserDataUpdated() { userNameView.text = ... profileImage.setImageUrl(...) } notifyUserDataUpdated() Dependency > Coupling > Message coupling

Slide 538

Slide 538 text

Coupling: Summary Avoid strong coupling - Use stamp/data coupling instead of content/common/control Dependency > Coupling > Summary

Slide 539

Slide 539 text

Coupling: Summary Avoid strong coupling - Use stamp/data coupling instead of content/common/control Content coupling: Relies on internal working Dependency > Coupling > Summary

Slide 540

Slide 540 text

Coupling: Summary Avoid strong coupling - Use stamp/data coupling instead of content/common/control Content coupling: Relies on internal working Common coupling: Depends on global singleton/variables Dependency > Coupling > Summary

Slide 541

Slide 541 text

Slide 542

Slide 542 text

Code←readability←session 7 Dependency ‐‐

Slide 543

Slide 543 text

Slide 544

Slide 544 text

Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Direction

Slide 545

Slide 545 text

Direction Keep the dependency in one direction as far as possible = It’s good if there’s no cycle Dependency > Direction

Slide 546

Slide 546 text

Preferred dependency direction: Example - Caller to callee - Detail to abstraction - Complex to simple - Algorithm to data model - Mutable to immutable - Frequently updated layer to stable layer Dependency > Direction

Slide 547

Slide 547 text

Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction

Slide 548

Slide 548 text

Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction

Slide 549

Slide 549 text

Bad code example class MediaViewPresenter { fun getVideoUri(): Uri = ... fun playVideo() { videoPlayerView.play(this) ... Dependency > Direction > Caller to callee

Slide 550

Slide 550 text

Bad code example class MediaViewPresenter { fun getVideoUri(): Uri = ... fun playVideo() { videoPlayerView.play(this) ... class VideoPlayerView { fun play(presenter: MediaViewPresenter) { val uri = presenter.getVideoUri() Dependency > Direction > Caller to callee

Slide 551

Slide 551 text

What is to be checked Call stack of the example - MediaViewPresenter.playVideo() - VideoPlayerView.play(...) - MediaViewPresenter.getVideoUri() Dependency > Direction > Caller to callee

Slide 552

Slide 552 text

What is to be checked Call stack of the example - MediaViewPresenter.playVideo() - VideoPlayerView.play(...) - MediaViewPresenter.getVideoUri() A call stack of A → B → A is a typical bad signal Dependency > Direction > Caller to callee

Slide 553

Slide 553 text

How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee

Slide 554

Slide 554 text

How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee

Slide 555

Slide 555 text

Pass values as parameters class MediaViewPresenter { fun getVideoUri(): Uri = ... fun playVideo() { videoPlayerView.play(this) ... class VideoPlayerView { fun play(presenter: MediaViewPresenter) { val uri = presenter.getVideoUri() Dependency > Direction > Caller to callee

Slide 556

Slide 556 text

Pass values as parameters class MediaViewPresenter { fun getVideoUri(): Uri = ... fun playVideo() { videoPlayerView.play(getVideoUri()) ... class VideoPlayerView { fun play(videoUri: Uri) { ... Dependency > Direction > Caller to callee

Slide 557

Slide 557 text

How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee

Slide 558

Slide 558 text

Extract small classes Can't pass value directly for asynchronous evaluation Dependency > Direction > Caller to callee

Slide 559

Slide 559 text

Extract small classes Can't pass value directly for asynchronous evaluation ┌────────────────────────┐ │ MediaViewPresenter │ │ │───────▶┌──────────────┐ │ getVideoUri() ◀──────┼────────│ VideoPlayer │ │ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee

Slide 560

Slide 560 text

Slide 561

Slide 561 text

Extract small classes: Step 1 Create a named class for the callback ┌────────────────────────┐ │ MediaViewPresenter │ │ │───────▶┌──────────────┐ │ getVideoUri() ◀──────┼────────│ VideoPlayer │ │ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee

Slide 562

Slide 562 text

Extract small classes: Step 1 Create a named class for the callback ┌────────────────────────┐ │ MediaViewPresenter │ │ ┌──────────────────┐ │───────▶┌──────────────┐ │ │ VideoUriProvider │◀──┼────────│ VideoPlayer │ │ └──────────────────┘ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee

Slide 563

Slide 563 text

Extract small classes: Step 2 Move the named class out of the outer class ┌────────────────────────┐ │ MediaViewPresenter │ │ ┌──────────────────┐ │───────▶┌──────────────┐ │ │ VideoUriProvider │◀──┼────────│ VideoPlayer │ │ └──────────────────┘ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee

Slide 564

Slide 564 text

Extract small classes: Step 2 Move the named class out of the outer class ┌────────────────────┐ │ MediaViewPresenter │─────┐ └────────────────────┘ │ ┌──────────────┐ │ └─────▶│ VideoPlayer │ │ └──────────────┘ ▼ │ ┌──────────────────┐ │ │ VideoUriProvider │◀────────────────────┘ └──────────────────┘ Dependency > Direction > Caller to callee

Slide 565

Slide 565 text

How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee

Slide 566

Slide 566 text

Dependency on caller: Exceptions Dependency on a caller may be acceptable for the following: Dependency > Direction > Caller to callee

Slide 567

Slide 567 text

Dependency on caller: Exceptions Dependency on a caller may be acceptable for the following: - Threads - Event listeners - Strategy pattern like functions (e.g., forEach) Dependency > Direction > Caller to callee

Slide 568

Slide 568 text

Dependency on caller: Exceptions Dependency on a caller may be acceptable for the following: - Threads - Event listeners - Strategy pattern like functions (e.g., forEach) Alternative options: Promise pattern, coroutine, reactive... Dependency > Direction > Caller to callee

Slide 569

Slide 569 text

Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Detail to abstraction

Slide 570

Slide 570 text

Dependency on detail Don't depend on inheritances = Don't check type of this or self Exception: Sealed class Dependency > Direction > Detail to abstraction

Slide 571

Slide 571 text

Bad code example open class IntList { fun addElement(value: Int) { if (this is ArrayIntList) { ... } else { ... ... class ArrayIntList: IntList() { ... Dependency > Direction > Detail to abstraction

Slide 572

Slide 572 text

What's wrong with the bad example Brakes encapsulation and not robust - For inheritance speciﬁcation change - For adding a new inheritance Dependency > Direction > Detail to abstraction

Slide 573

Slide 573 text

Fixed code example Remove downcast with overriding open class IntList { open fun addElement(value: Int) { ... class ArrayIntList: IntList() { override fun addElement(value: Int) { ... Dependency > Direction > Detail to abstraction

Slide 574

Slide 574 text

Dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Complex to simple

Slide 575

Slide 575 text

Bad code example class UserData( val userId: UserId, ... ) class UserDataRequester { fun obtainFromServer(): UserData { ... Dependency > Direction > Complex to simple

Slide 576

Slide 576 text

Bad code example class UserData( val userId: UserId, ... val requester: UserDataRequester ) class UserDataRequester { fun obtainFromServer(): UserData { ... Dependency > Direction > Complex to simple

Slide 577

Slide 577 text

What's wrong with the bad example Unnecessary dependency may cause a bug - May leak requester resource - May make inconsistent state if there are multiple requesters Dependency > Direction > Complex to simple

Slide 578

Slide 578 text

What's wrong with the bad example Unnecessary dependency may cause a bug - May leak requester resource - May make inconsistent state if there are multiple requesters A simple type may be used widely and passed to other modules Dependency > Direction > Complex to simple

Slide 579

Slide 579 text

How to remove dependency on complex class Just remove dependency of "simple to complex" - Pass a complex class instance directly if required class UserData( val userId: UserId, ... ) Dependency > Direction > Complex to simple

Slide 580

Slide 580 text

Exceptions It's sometimes necessary to depend on complex types - Mediator pattern - Adapter or facade - Data binder Dependency > Direction > Complex to simple

Slide 581

Slide 581 text

Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Summary

Slide 582

Slide 582 text

Direction: Summary Maintain dependency in one direction - From caller to callee - From complex, detailed, large to simple, abstract, small Exceptions - Async call, sealed class, mediator pattern Dependency > Direction > Summary

Slide 583

Slide 583 text

Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Redundancy

Slide 584

Slide 584 text

Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy

Slide 585

Slide 585 text

Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy

Slide 586

Slide 586 text

Cascaded dependency Avoid unnecessary dependency chain Dependency > Redundancy > Cascaded dependency

Slide 587

Slide 587 text

Cascaded dependency Avoid unnecessary dependency chain ┌──────────────────────┐ │ MessageTextPresenter │ └──────────────────────┘ │ has instance ▼ ┌──────────────────────┐ │ TimestampPresenter │ └──────────────────────┘ │ has instance ▼ ┌──────────────────────┐ │ MessageDataProvider │ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency

Slide 588

Slide 588 text

Cascaded dependency Avoid unnecessary dependency chain ┌──────────────────────┐ depending indirectly │ MessageTextPresenter │─ ─ ─ ─ ─ ─ ─ ─ ┐ └──────────────────────┘ │ has instance │ ▼ (only for MessageDataProvider) ┌──────────────────────┐ │ │ TimestampPresenter │ └──────────────────────┘ │ │ has instance ▼ │ ┌──────────────────────┐ │ MessageDataProvider │◀ ─ ─ ─ ─ ─ ─ ─ ┘ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency

Slide 589

Slide 589 text

Bad cascaded dependency example class TimestampPresenter { val dataProvider: MessageDataProvider = ... fun invalidateViews() { // Update timestamp by `dataProvider` ... Dependency > Redundancy > Cascaded dependency

Slide 590

Slide 590 text

Bad cascaded dependency example class TimestampPresenter { val dataProvider: MessageDataProvider = ... fun invalidateViews() { // Update timestamp by `dataProvider` ... class MessageTextPresenter { val timestampPresenter: TimestampPresenter = ... fun invalidateViews() { val messageData = timestampPresenter.dataProvider... // Update message text view by `messageData` Dependency > Redundancy > Cascaded dependency

Slide 591

Slide 591 text

What's wrong with the bad example Unnecessary dependency - MessageTextPresenter is unrelated with TimestampPresenter Indirect dependency - MessageTextPresenter does not have MessageDataProvider directly Dependency > Redundancy > Cascaded dependency

Slide 592

Slide 592 text

How to ﬁx cascaded dependency Flatten nested dependencies by means of direct dependencies ┌──────────────────────┐ depending indirectly │ MessageTextPresenter │─ ─ ─ ─ ─ ─ ─ ─ ┐ └──────────────────────┘ │ has instance │ ▼ (only for MessageDataProvider) ┌──────────────────────┐ │ │ TimestampPresenter │ └──────────────────────┘ │ │ has instance ▼ │ ┌──────────────────────┐ │ MessageDataProvider │◀ ─ ─ ─ ─ ─ ─ ─ ┘ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency

Slide 593

Slide 593 text

How to ﬁx cascaded dependency Flatten nested dependencies by means of direct dependencies ┌──────────────────────┐ │ MessageTextPresenter │──┐ └──────────────────────┘ │ │ has instance ┌──────────────────────┐ ├──────────────▶│ MessageDataProvider │ ┌──────────────────────┐ │ └──────────────────────┘ │ TimestampPresenter │──┘ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency

Slide 594

Slide 594 text

Fixed example of cascaded dependency class TimestampPresenter { val dataProvider: MessageDataProvider = ... fun invalidateViews() { // Update timestamp by `dataProvider` ... class MessageTextPresenter { val dataProvider: MessageDataProvider = ... fun invalidateViews() { // Update message text view by `dataProvider` Dependency > Redundancy > Cascaded dependency

Slide 595

Slide 595 text

Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy > Dependency set

Slide 596

Slide 596 text

Redundant dependency set Avoid duplication of "depending class set" ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ ├───────────────────────────┘ │ │ ▼ ┌─────────────────────────┐ │ MessageDataProvider │ └─────────────────────────┘ Dependency > Redundancy > Dependency set

Slide 597

Slide 597 text

Redundant dependency set Avoid duplication of "depending class set" ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ │ └ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┼ ─ ─ ┤ ├───────────────────────────┘ │ │ │ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set

Slide 598

Slide 598 text

What's wrong with redundant dependency set? Fragile for modiﬁcation - When a new depending/depended type is added Duplicates switching logic - Provider selection/fallback logic Dependency > Redundancy > Dependency set

Slide 599

Slide 599 text

How to remove redundant dependency set Create an intermediate layer to consolidate dependency set ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ │ └ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┼ ─ ─ ┤ ├───────────────────────────┘ │ │ │ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set

Slide 600

Slide 600 text

How to remove redundant dependency set Create an intermediate layer to consolidate dependency set ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ └────────────────┬────────────────┘ ▼ ┌─────────────────────────┐ │ MessageDataProvider │ └─────────────────────────┘ ┌────────────────┴────────────────┐ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set

Slide 601

Slide 601 text

Caution to remove redundant dependency Don’t create a layer if you don’t need it now - Keep "YAGNI" and "KISS" in mind Don't provide access to a hidden class - Never create MessageDataProvider.serverMessageDataProvider Dependency > Redundancy > Dependency set

Slide 602

Slide 602 text

Redundancy: Summary - Remove dependencies only for providing another dependency - Add layer to remove dependency set duplication Dependency > Redundancy > Summary

Slide 603

Slide 603 text

Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Explicitness

Slide 604

Slide 604 text

Implicit dependency A dependency does not appear on a class diagram Dependency > Explicitness

Slide 605

Slide 605 text

Implicit dependency A dependency does not appear on a class diagram Avoid implicit dependencies, use explicit ones instead Dependency > Explicitness

Slide 606

Slide 606 text

Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness

Slide 607

Slide 607 text

Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness > Implicit data domain

Slide 608

Slide 608 text

Bad code example Question: What's wrong with the following function? fun setViewBackgroundColor(colorString: String) { val colorCode = when(colorString) { "red" -> 0xFF0000 "green" -> 0x00FF00 else -> 0x000000 } view.setBackgroundColor(colorCode) } Dependency > Explicitness > Implicit data domain

Slide 609

Slide 609 text

Why implicit data domain is bad Hard to ﬁnd which value is accepted - "blue", "RED", "FF0000" are invalid Dependency > Explicitness > Implicit data domain

Slide 610

Slide 610 text

Why implicit data domain is bad Hard to ﬁnd which value is accepted - "blue", "RED", "FF0000" are invalid Color string conversion logic is tightly coupled with the function - Easy to break if we add/remove a color Dependency > Explicitness > Implicit data domain

Slide 611

Slide 611 text

How to remove implicit data domain Create a data type representing the data domain enum class BackgroundColor(val colorCode: Int) { RED(0xFF0000), GREEN(0x00FF00) } Dependency > Explicitness > Implicit data domain

Slide 612

Slide 612 text

How to remove implicit data domain Create a data type representing the data domain enum class BackgroundColor(val colorCode: Int) { RED(0xFF0000), GREEN(0x00FF00) } fun setViewBackgroundColor(color: BackgroundColor) { view.setBackgroundColor(color.colorCode) } Dependency > Explicitness > Implicit data domain

Slide 613

Slide 613 text

Other options instead of deﬁning a type Options to consider for an invalid value - Just do nothing - Return an error value - Throw a runtime error (not recommended) Dependency > Explicitness > Implicit data domain

Slide 614

Slide 614 text

Other options instead of deﬁning a type Options to consider for an invalid value - Just do nothing - Return an error value - Throw a runtime error (not recommended) Note 1: Documentation is required for any option Dependency > Explicitness > Implicit data domain

Slide 615

Slide 615 text

Slide 616

Slide 616 text

Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness > Subtype behavior

Slide 617

Slide 617 text

Implicit dependency by subtype A caller depends on the inherited type implicitly ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit │ │ │ │ ┌───────────────────────┐ │ListenerImplementation │ └───────────────────────┘ Dependency > Explicitness > Subtype behavior

Slide 618

Slide 618 text

Implicit dependency by subtype A caller depends on the inherited type implicitly ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit ┌─────┼───────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ListenerImplementation │ │ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior

Slide 619

Slide 619 text

Implicit dependency example by subtype interface ListenerInterface { fun queryInt(): Int } class LargeClass { inner class ListenerImplementation : ListenerInterface { ... } } class Caller(listener : ListenerInterface) { ... } Dependency > Explicitness > Subtype behavior

Slide 620

Slide 620 text

What's wrong with subtype Depends on result of the function call on LargeClass implicitly - Does not appear on class diagram - Hard to ﬁnd Caller relying on LargeClass implementation detail Dependency > Explicitness > Subtype behavior

Slide 621

Slide 621 text

How to remove implicit dependency Remove the interface if there is only one implementation ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit ┌─────┼───────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ListenerImplementation │ │ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior

Slide 622

Slide 622 text

How to remove implicit dependency Remove the interface if there is only one implementation call ┌───────────────────────┐ ┌─────│ Caller │ │ └───────────────────────┘ │ │ ┌─────────────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ │ Listener │◀─┼─┘ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior

Slide 623

Slide 623 text

Fixed code example class LargeClass { inner class Listener { fun ... } } class Caller(listener : Listener) { ... } Dependency > Explicitness > Subtype behavior

Slide 624

Slide 624 text

Explicitness: Summary Make dependencies visible on a class diagram - Deﬁne types to represent data domain - Remove unnecessary interface Dependency > Explicitness > Summary

Slide 625

Slide 625 text

Summary - Coupling: Relax content/common/control coupling - Direction: Remove cyclic dependency as far as possible - Redundancy: Don't cascade or duplicate dependency set - Explicitness: Make dependency visible on a class diagram Dependency > Summary

Slide 626

Slide 626 text

Code-Readability / Session-8 Review #1

Slide 627

Slide 627 text

Slide 628

Slide 628 text

Why do we need code review? Review > Introduction

Slide 629

Slide 629 text

Why do we need code review? Most important: Readability - Approve code change only when it is clean enough Review > Introduction

Slide 630

Slide 630 text

Why do we need code review? Most important: Readability - Approve code change only when it is clean enough Nice to have: Logic correctness - Edge cases, illegal state, race condition, exceptions... - Author's responsibility Review > Introduction

Slide 631

Slide 631 text

Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Introduction

Slide 632

Slide 632 text

Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Create PR

Slide 633

Slide 633 text

How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR

Slide 634

Slide 634 text

How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR

Slide 635

Slide 635 text

Commit structure - Squash unnecessary commits - Don't put logical change and syntax change into a commit - Think "cut of axis" for commits Review > Create PR > Commit structure

Slide 636

Slide 636 text

Commit structure - Squash unnecessary commits - Don't put logical change and syntax change into a commit - Think "cut of axis" for commits Review > Create PR > Commit structure > Squash unnecessary commits

Slide 637

Slide 637 text

Unnecessary commits Remove or squash "reverting commit" Review > Create PR > Commit structure > Squash unnecessary commits

Slide 638

Slide 638 text

Unnecessary commits Remove or squash "reverting commit" Bad example: Commit 1: Add debug log Commit 2: Implement a method of feature X Commit 3: Remove debug log Review > Create PR > Commit structure > Squash unnecessary commits

Slide 639

Slide 639 text

How to remove unnecessary commits Use git rebase -i or similar command Review > Create PR > Commit structure > Squash unnecessary commits

Slide 640

Slide 640 text

How to remove unnecessary commits Use git rebase -i or similar command Fixed example: Commit 2: Implement a method of feature X Review > Create PR > Commit structure > Squash unnecessary commits

Slide 641

Slide 641 text

Commit structure - Squash unnecessary commits - Don't put logical change and syntax change into a commit - Think "cut of axis" for commits Review > Create PR > Commit structure > Logical and syntax change

Slide 642

Slide 642 text

Logical and syntax change in a commit Typical anti-patterns - IDE auto clean up + actual refactoring - Extracting as a function + function body refactoring - Renaming + non-trivial parameter type change Review > Create PR > Commit structure > Logical and syntax change

Slide 643

Slide 643 text

Logical and syntax change in a commit Typical anti-patterns - IDE auto clean up + actual refactoring - Extracting as a function + function body refactoring - Renaming + non-trivial parameter type change If it's hard to read, consider splitting the commit Review > Create PR > Commit structure > Logical and syntax change

Slide 644

Slide 644 text

Commit structure - Squash unnecessary commits - Don't put logical change and syntax change into a commit - Think "cut of axis" for commits Review > Create PR > Commit structure > Cut of axis

Slide 645

Slide 645 text

Example of splitting PR class Deprecated { fun functionA() { /* ... */ } fun functionB() { /* ... */ } } class New { /* ... */ } Review > Create PR > Commit structure > Cut of axis

Slide 646

Slide 646 text

Example of splitting PR class Deprecated { fun functionA() { /* ... */ } fun functionB() { /* ... */ } } class New { /* ... */ } Objective: Remove class Deprecated with moving the functions into class New Review > Create PR > Commit structure > Cut of axis

Slide 647

Slide 647 text

Axis options to consider Option A: Commit 1: Define New.functionA/functionB Commit 2: Replace receivers Deprecated with New Commit 3: Remove class Deprecated Review > Create PR > Commit structure > Cut of axis

Slide 648

Slide 648 text

Axis options to consider Option A: Commit 1: Define New.functionA/functionB Commit 2: Replace receivers Deprecated with New Commit 3: Remove class Deprecated Option B: Commit 1: Move Deprecated.functionA to New, and update the callers Commit 2: Move Deprecated.functionB to New, and update the callers Commit 3: Remove class Deprecated Review > Create PR > Commit structure > Cut of axis

Slide 649

Slide 649 text

Better option for axis Review > Create PR > Commit structure > Cut of axis

Slide 650

Slide 650 text

Better option for axis Option B is better than A Review > Create PR > Commit structure > Cut of axis

Slide 651

Slide 651 text

Better option for axis Option B is better than A Reason: - Easy to understand the intention - Veriﬁable that there is no logical change - Traceable commit history Review > Create PR > Commit structure > Cut of axis

Slide 652

Slide 652 text

How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR > Keep PR small

Slide 653

Slide 653 text

Keep your PR small - Split your daily work into several PRs - Create supporting PRs - Specify the scope of a PR Review > Create PR > Keep PR small

Slide 654

Slide 654 text

Keep your PR small - Split your daily work into several PRs - Create supporting PRs - Specify the scope of a PR Review > Create PR > Keep PR small > Split daily work

Slide 655

Slide 655 text

Split your daily work into several PRs Don't create a large PR like "PR of the week" - Hard to read for reviewers - Makes review iteration long Two ways to split PRs: Top-down, Bottom-up Review > Create PR > Keep PR small > Split daily work

Slide 656

Slide 656 text

Top-down approach Fill implementation detail later Review > Create PR > Keep PR small > Split daily work

Slide 657

Slide 657 text

Top-down approach Fill implementation detail later 1. Create skeleton classes to explain the structure 2. Explain future plan with TODO and PR comments Review > Create PR > Keep PR small > Split daily work

Slide 658

Slide 658 text

Top-down approach Fill implementation detail later 1. Create skeleton classes to explain the structure 2. Explain future plan with TODO and PR comments class UserProfilePresenter( val userProfileUseCase: UseCase val profileRootView: View ) { fun showProfileImage() = TODO(...) fun addUserTag() = TODO(...) Review > Create PR > Keep PR small > Split daily work

Slide 659

Slide 659 text

Bottom-up approach Create small parts ﬁrst Review > Create PR > Keep PR small > Split daily work

Slide 660

Slide 660 text

Bottom-up approach Create small parts ﬁrst 1. Implement simple types/procedure without any client code 2. Explain future plan of client code with PR comments Review > Create PR > Keep PR small > Split daily work

Slide 661

Slide 661 text

Bottom-up approach Create small parts ﬁrst 1. Implement simple types/procedure without any client code 2. Explain future plan of client code with PR comments data class UserProfileData(val ..., val ...) object UserNameStringUtils { fun String.normalizeEmoji(): String = ... fun isValidUserName(userName: String): Boolean = ... Review > Create PR > Keep PR small > Split daily work

Slide 662

Slide 662 text

Keep your PR small - Split your daily work into several PRs - Create supporting PRs - Specify the scope of a PR Review > Create PR > Keep PR small > Create supporting PRs

Slide 663

Slide 663 text

Create supporting PRs Question: How can you make PRs with the following situation? 1. You made commits of class New 2. You found New has code duplication with Old 3. You need a large change to consolidate the logic (>100 LOC) - Because there are already a lot of callers of Old Review > Create PR > Keep PR small > Create supporting PRs

Slide 664

Slide 664 text

Worst way to create a PR Commit1: Create a class "New" Commit2: Extract duplicated code of "New" and "Old" # >100 LOC Problem: The PR is large, and has two responsibilities Review > Create PR > Keep PR small > Create supporting PRs

Slide 665

Slide 665 text

Better way to create a PR PR1: Commit1: Create a class "New" PR2 (including PR1): Commit2: Extract duplicated code of "New" and "Old" # >100 LOC Problem: There is temporary code duplication - We may forget to create PR2 Review > Create PR > Keep PR small > Create supporting PRs

Slide 666

Slide 666 text

Best way to create a PR 1/3 Reorder, split, merge commits 1: Split Commit2 for New and Old Commit1: Create a class "New" Commit2_New: Extract code of "New" Commit2_Old: Extract code of "Old" Review > Create PR > Keep PR small > Create supporting PRs

Slide 667

Slide 667 text

Best way to create a PR 2/3 2: Move Commit2_Old to the ﬁrst Commit2_Old: Extract code of "Old" Commit1: Create a class "New" Commit2_New: Extract code of "New" Review > Create PR > Keep PR small > Create supporting PRs

Slide 668

Slide 668 text

Best way to create a PR 3/3 3: Squash commits Commit1 and Commit2_New Commit2_Old: Extract code of "Old" Commit1: Create a class "New" 4: Create PRs for Commit2_Old and Commit1 Review > Create PR > Keep PR small > Create supporting PRs

Slide 669

Slide 669 text

Keep your PR small - Split your daily work into several PRs - Create supporting PRs - Specify the scope of a PR Review > Create PR > Keep PR small > Specify scope of PR

Slide 670

Slide 670 text

Conﬂict between author and reviewer Author's responsibility: - Keep the PR small Review > Create PR > Keep PR small > Specify scope of PR

Slide 671

Slide 671 text

Conﬂict between author and reviewer Author's responsibility: - Keep the PR small Reviewer's responsibility: - Don't approve if there is room to improve Review > Create PR > Keep PR small > Specify scope of PR

Slide 672

Slide 672 text

Conﬂict between author and reviewer Author's responsibility: - Keep the PR small Reviewer's responsibility: - Don't approve if there is room to improve These two responsibilities may conﬂict with each other Review > Create PR > Keep PR small > Specify scope of PR

Slide 673

Slide 673 text

Describe scope of PR Write PR comment and TODO comment to explain: - The main purpose - Future plan - Out of scope Review > Create PR > Keep PR small > Specify scope of PR

Slide 674

Slide 674 text

How to create PR: Summary Make readable PR that is structured and small - Make the objective of a PR/commits clear - Squash, split, reorder commits Review > Create PR > Summary

Slide 675

Slide 675 text

Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Address review comment

Slide 676

Slide 676 text

Important point in addressing comments Review > Address review comment

Slide 677

Slide 677 text

Important point in addressing comments Don't just address comments Review > Address review comment

Slide 678

Slide 678 text

Don't just address comments - Think why reviewer asked/misunderstood - Understand reviewer's suggestion - Consider addressing a comment to other parts - Don't make reviewers repeat Review > Address review comment

Slide 679

Slide 679 text

Slide 680

Slide 680 text

Think why reviewer asked/misunderstood Typical signal that the code is unreadable or wrong Review > Address review comment

Slide 681

Slide 681 text

Think why reviewer asked/misunderstood Typical signal that the code is unreadable or wrong Example of comment "Why do you check null here?": - Remove the null check if unnecessary - Write inline comment to explain why - Extract as a local value to explain (e.g., isUserExpired) Review > Address review comment

Slide 682

Slide 682 text

Slide 683

Slide 683 text

Understand reviewer's suggestion Don't just paste attached code in a review comment - Think what is the key idea - Consider renaming - Conﬁrm it's logically correct: exceptions, race conditions ... Review > Address review comment

Slide 684

Slide 684 text

Slide 685

Slide 685 text

Consider addressing a comment to other parts Find similar code and address it Review > Address review comment

Slide 686

Slide 686 text

Consider addressing a comment to other parts Find similar code and address it Example of comment "Add @Nullable to this parameter": - Apply to other parameters - Apply to the return value - Apply to other functions Review > Address review comment

Slide 687

Slide 687 text

Slide 688

Slide 688 text

Don't make reviewers repeat Check what you asked previously before sending a review request - Coding style and conventions - Natural language: word choice, grammar - Language/platform idiom - Conditional branch structure - Testing Review > Address review comment

Slide 689

Slide 689 text

Don't just address comment: Summary Understand what is the intention of a review comment - Find the key idea and reason of the comment - Try to apply it to other parts - Relieve reviewers' workload Review > Address review comment

Slide 690

Slide 690 text

Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Reviewer principles

Slide 691

Slide 691 text

Principles for reviewers Don't be too kind Review > Reviewer principles

Slide 692

Slide 692 text

Principles for reviewers Don't be too kind (but be kind a little) Review > Reviewer principles

Slide 693

Slide 693 text

Principles for reviewers - Don't neglect review requests - Reject problematic PR - Don't guess author's situation - Consider other options of "answer" Review > Reviewer principles

Slide 694

Slide 694 text

Principles for reviewers - Don't neglect review requests - Reject problematic PR - Don't guess author's situation - Consider other options of "answer" Review > Reviewer principles > Don't neglect

Slide 695

Slide 695 text

Don't neglect review requests Deﬁne a reply time limit for the project - e.g., 24 hours of working days Question: What is the worst reaction to a review request? Review > Reviewer principles > Don't neglect

Slide 696

Slide 696 text

Reactions to review requests GOOD: NOT SO GOOD: - Simply ignore WORST: Review > Reviewer principles > Don't neglect

Slide 697

Slide 697 text

Reactions to review requests GOOD: NOT SO GOOD: - Simply ignore WORST: - Reply "I'll review" without review Review > Reviewer principles > Don't neglect

Slide 698

Slide 698 text

Reactions to review requests GOOD: - Review soon - Redirect to other reviewer - "I cannot review", "I'll be late"... NOT SO GOOD: - Simply ignore WORST: - Reply "I'll review" without review Review > Reviewer principles > Don't neglect

Slide 699

Slide 699 text

When you cannot review If you are busy, reply so Review > Reviewer principles > Don't neglect

Slide 700

Slide 700 text

When you cannot review If you are busy, reply so The author can: - Wait for the PR review if it doesn't block other tasks - Find other reviewers if it's an urgent PR Review > Reviewer principles > Don't neglect

Slide 701

Slide 701 text

Principles for reviewers - Don't neglect review requests - Reject problematic PR - Don't guess author's situation - Consider other options of "answer" Review > Reviewer principles > Reject problematic PR

Slide 702

Slide 702 text

Reject problematic PR Don't spend too much time reviewing a problematic PR Review > Reviewer principles > Reject problematic PR

Slide 703

Slide 703 text

Reject problematic PR Ask the author to re-create a PR without reading the detail Review > Reviewer principles > Reject problematic PR

Slide 704

Slide 704 text

Reject problematic PR Ask the author to re-create a PR without reading the detail Too large or hard to read: - Ask to split Review > Reviewer principles > Reject problematic PR

Slide 705

Slide 705 text

Reject problematic PR Ask the author to re-create a PR without reading the detail Too large or hard to read: - Ask to split Totally wrong on purpose, assumption, or structure: - Ask to make skeleton classes or have casual chat Review > Reviewer principles > Reject problematic PR

Slide 706

Slide 706 text

Slide 707

Slide 707 text

Don't guess author's situation Reviewers should not care about deadline - Let them explain if they want to merge soon - Need to ﬁle issue tickets, and add TODO at least Review > Reviewer principles > Don't guess author's situation

Slide 708

Slide 708 text

Don't guess author's situation Reviewers should not care about deadline - Let them explain if they want to merge soon - Need to ﬁle issue tickets, and add TODO at least Exception: Major/critical issues on a release branch or hot-ﬁx - Tests are still required - Reviewers should keep cooler than authors Review > Reviewer principles > Don't guess author's situation

Slide 709

Slide 709 text

Slide 710

Slide 710 text

Options of a review comment The answer is not only showing answer Review > Reviewer principles > Review comment options

Slide 711

Slide 711 text

Options of a review comment The answer is not only showing answer To save reviewer's time and encourage authors' growth: - Ask questions to make the author come up with an idea - Suggest options to make the author decide - Leave a note to share knowledge Review > Reviewer principles > Review comment options

Slide 712

Slide 712 text

Principles for reviewers: Summary Don't be too kind, but help authors - Reply if you can't review - Fine to refuse problematic PR Review > Reviewer principles > Summary

Slide 713

Slide 713 text

Principles for reviewers: Summary Don't be too kind, but help authors - Reply if you can't review - Fine to refuse problematic PR Think what is the best comment - including time cost and author's growth Review > Reviewer principles > Summary

Slide 714

Slide 714 text

Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Reviewer principles

Slide 715

Slide 715 text

What should be commented - Styles, conventions, idioms ... (CI can review instead) - Tests - All in this presentation series - Principles, natural language, structure, dependency - Any issues you found Review > Comment contents

Slide 716

Slide 716 text

Slide 717

Slide 717 text

Code review case study Adding a parameter to function Review > Comment contents > Case study

Slide 718

Slide 718 text

Code review case study Adding a parameter to function fun showPhotoView( photoData: PhotoData ) { if (!photoData.isValid) { return } ... Review > Comment contents > Case study

Slide 719

Slide 719 text

Code review case study Adding a parameter to function fun showPhotoView( photoData: PhotoData, isFromEditor: Boolean ) { if (!photoData.isValid) { if (isFromEditor) { showDialog() } return } ... Review > Comment contents > Case study

Slide 720

Slide 720 text

Code review case study Naming: Describe what it is rather than how it is used isFromEditor -> showsDialogOnError Review > Comment contents > Case study

Slide 721

Slide 721 text

Code review case study Naming: Describe what it is rather than how it is used fun showPhotoView( photoData: PhotoData, isFromEditor: Boolean ) { if (!photoData.isValid) { if (isFromEditor) { showDialog() } return } ... Review > Comment contents > Case study

Slide 722

Slide 722 text

Code review case study Naming: Describe what it is rather than how it is used fun showPhotoView( photoData: PhotoData, showsDialogOnError: Boolean ) { if (!photoData.isValid) { if (showsDialogOnError) { showDialog() } return } ... Review > Comment contents > Case study

Slide 723

Slide 723 text

Code review case study Dependency: Control coupling with a boolean ﬂag Extract showDialog call Review > Comment contents > Case study

Slide 724

Slide 724 text

Code review case study Dependency: Control coupling with a boolean ﬂag Extract showDialog call - For editor: val isViewShown = showPhotoView(...) if (!isViewShown) { showErrorDialog(...) } - For other place: showPhotoView(...) Review > Comment contents > Case study

Slide 725

Slide 725 text

Code review case study Dependency: Control coupling with a boolean ﬂag fun showPhotoView(photoData: PhotoData): Boolean { if (!photoData.isValid) { return false } ... Review > Comment contents > Case study

Slide 726

Slide 726 text

Code review case study Comments: Comment if a function has both side-effect and return value Add comment to explain the return value Review > Comment contents > Case study

Slide 727

Slide 727 text

Code review case study Comments: Comment if a function has both side-effect and return value /** * Shows a photo at the center if the given photo data is ..., * and returns true. * Otherwise, this returns false without showing the view. */ fun showPhotoView(photoData: PhotoData): Boolean { if (!photoData.isValid) { return false } Review > Comment contents > Case study

Slide 728

Slide 728 text

What we comment: Summary Review the following items - Styles, conventions, manners, idioms ... - Principles, natural language, structure, dependency ... Review > Comment contents > Summary

Slide 729

Slide 729 text

What we comment: Summary Review the following items - Styles, conventions, manners, idioms ... - Principles, natural language, structure, dependency ... Read over "code readability" presentation Review > Comment contents > Summary

Slide 730

Slide 730 text

Summary Review is for readability Review > Summary

Slide 731

Slide 731 text

Summary Review is for readability Author: - Keep PR small and structured - Understand review comments, don't just address Review > Summary

Slide 732

Slide 732 text

Summary Review is for readability Author: - Keep PR small and structured - Understand review comments, don't just address Reviewer: - Don't be too kind, you can ask Review > Summary