Code readability

**The new version is available!**
https://gist.github.com/munetoshi/65a1b563fb2c271f328c121a4ac63571

This deck is old version.
---

Session 1: Introduction and Principles
- https://speakerdeck.com/munetoshi/code-readability?slide=2

Session 2: Naming
- https://speakerdeck.com/munetoshi/code-readability?slide=75

Session 3: Comments
- https://speakerdeck.com/munetoshi/code-readability?slide=160

Session 4: State
- https://speakerdeck.com/munetoshi/code-readability?slide=240

Session 5: Procedure
- https://speakerdeck.com/munetoshi/code-readability?slide=338

Session 6: Dependency I
- https://speakerdeck.com/munetoshi/code-readability?slide=442

Session 7: Dependency II
- https://speakerdeck.com/munetoshi/code-readability?slide=542

Session 8: Review
- https://speakerdeck.com/munetoshi/code-readability?slide=626

© Munetoshi Ishikawa 2019, supported by LINE corporation

1. Code Readability Munetoshi Ishikawa
2. Code readability session 1 Introduction and Principles
3. 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
4. Why we need readable code Introduction and Principles > Introduction
5. Why we need readable code Sustainable development - To extend saturation curve of development Introduction and Principles > Introduction
6. 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 fix with a few lines Introduction and Principles > Introduction
7. Optimize for sustainable development Focus on team productivity Introduction and Principles > Introduction
8. 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
9. 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
10. Prisoner's dilemma Team productivity may decrease if we focus on the personal Introduction and Principles > Introduction
11. 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
12. Learn how to write readable code Feature implementation itself is easy Special training is not required Introduction and Principles > Introduction
13. 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
14. 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
15. 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
16. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles
17. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles
18. 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
19. 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
20. 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
21. 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
22. Don'ts for the boy scout rule 2/2 Don't add something without thought Introduction and Principles > Principles > The boy scout rule
23. 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
24. 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
25. Example of "don'ts" 1/2 Question: Is it fine 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
26. Example of "don'ts" 2/2 Answer: No Introduction and Principles > Principles > The boy scout rule
27. 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
28. 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
29. 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
30. 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
31. 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
32. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles
33. 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
34. 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
35. 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
36. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS
37. 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
38. 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 Beautiful code is not always readable Introduction and Principles > Principles > KISS
39. KISS: Good example fun getActualDataSingle(): Single<List<Int>> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) Introduction and Principles > Principles > KISS
40. KISS: Good example fun getActualDataSingle(): Single<List<Int>> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single<List<Int>> = Single.just(listOf(1, 10, 100)) Introduction and Principles > Principles > KISS
41. KISS: Bad example 1/2 fun getActualDataSingle(): Single<List<Int>> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single<List<Int>> = Single .fromCallable { listOf(1, 10, 100) } .subscribeOn(ioScheduler) Makes the code complex for unnecessary consistency Introduction and Principles > Principles > KISS
42. KISS: Bad example 2/2 fun getActualDataSingle(): Single<List<Int>> = Single .fromCallable(dataProvider::provide) .subscribeOn(ioScheduler) fun getDummyDataSingle(): Single<List<Int>> = 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
43. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS
44. Single method == small responsibility? Introduction and Principles > Principles > Single responsibility principles
45. Single method == small responsibility? No, definitely! 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
46. Single responsibility principle A class should have only one reason to change. — Robert C. Martin Introduction and Principles > Principles > Single responsibility principles
47. 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
48. Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList<Int>, val bookNames: MutableList<String>, ) { ... Introduction and Principles > Principles > Single responsibility principles
49. Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList<Int>, val bookNames: MutableList<String>, val bookIdToRenterNameMap: MutableMap<Int, String>, ) { ... Introduction and Principles > Principles > Single responsibility principles
50. Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList<Int>, val bookNames: MutableList<String>, val bookIdToRenterNameMap: MutableMap<Int, String>, val bookIdToDueDateMap: MutableMap<Int, Date>, ... ) { ... Introduction and Principles > Principles > Single responsibility principles
51. Single responsibility principle: Bad example class LibraryBookRentalData( val bookIds: MutableList<Int>, val bookNames: MutableList<String>, val bookIdToRenterNameMap: MutableMap<Int, String>, val bookIdToDueDateMap: MutableMap<Int, Date>, ... ) { fun findRenterName(bookName: String): String? fun findDueDate(bookName: String): Date? ... Introduction and Principles > Principles > Single responsibility principles
52. Single responsibility principle: What is wrong Introduction and Principles > Principles > Single responsibility principles
53. Single responsibility principle: What is wrong BookRentalData has all data of book, user, and circulation record Introduction and Principles > Principles > Single responsibility principles
54. 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
55. 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
56. 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<BookData, Entry> ) { data class Entry(val renter: UserData, val dueDate: Date) Introduction and Principles > Principles > Single responsibility principles
57. 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
58. How to confirm responsibility List what the type does, and try to summarize it Introduction and Principles > Principles > Single responsibility principles
59. How to confirm 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
60. Topics - Introduction - The boy scout rule - YAGNI - KISS - Single responsibility principle - Premature optimization is the root of all evil Introduction and Principles > Principles > KISS
61. Premature optimization 1/2 We should forget about small efficiencies, 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
62. 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
63. Example of good optimization 1/2 Before: val data = arrayList.firstOrNull { data -> data.key == expectedKey } Introduction and Principles > Principles > Premature optimization
64. 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
65. 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
66. Examples of bad optimization Don't optimize without profiling or platform support, either - Mutable instance reusing - Lazy initialization - Cache - Inline extraction - Instance pool Introduction and Principles > Principles > Premature optimization
67. Drawbacks of optimization - May obstruct simplification - 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
68. Required actions before optimization - Ask yourself "Do I really need it?" or "Is it easy enough to implement?" - Profile Target (time, memory), number, rate Introduction and Principles > Principles > Premature optimization
69. Summary Focus on sustainable development: Readability Introduction and Principles > Summary
70. Summary Focus on sustainable development: Readability The boy scout rule: Clean code whenever you have touched it Introduction and Principles > Summary
71. 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
72. 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 Introduction and Principles > Summary
73. 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
74. 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 Premature optimization: Profile or estimate before optimization Introduction and Principles > Summary
75. codeReadabilitySession2 N_A_M_I_N_G
76. 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 Naming > Introduction
77. What we name - Type: class, interface, enum, struct, protocol, trait - Value: property, field, parameter, local value - Procedure: function, method, subroutine - Scope: package, module, namespace - Resource: file, directory, ID - etc. Naming > Introduction
78. 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
79. How to create a good name - Use correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Introduction
80. Topics - Use correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Introduction
81. Why grammar is important Question 1: What is ListenerEventMessageClickViewText? Naming > Use the correct grammar
82. 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
83. 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
84. 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? Answer: A listener of click events on a message text view Naming > Use the correct grammar
85. Use correct grammar "Grammar" depends on the language and convention Let's look at the case of Java/Kotlin Naming > Use the correct grammar
86. Types of names 1/2 - Noun: Type, value (including property function) imageView, HashSet, indexOf - Imperative: Procedure findMessage, compareTo Naming > Use the correct grammar
87. 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
88. Grammar: Nouns Place the essential words at the last (= the word telling what it is) Naming > Use the correct grammar
89. Grammar: Nouns Place the essential words at the last (= the word telling what it is) ! : MessageEventHandler, buttonHeight Naming > Use the correct grammar
90. 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
91. 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
92. 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 Exception: Property function with a preposition e.g., indexOf(value), maxValueIn(array) Naming > Use the correct grammar
93. Grammar: Imperative Place a verb at the first Naming > Use the correct grammar
94. Grammar: Imperative Place a verb at the first "get X": ! getX, " xGet Naming > Use the correct grammar
95. Grammar: Imperative Place a verb at the first "get X": ! getX, " xGet "post Y": ! postY, " yPost Naming > Use the correct grammar
96. Grammar: Imperative Place a verb at the first "get X": ! getX, " xGet "post Y": ! postY, " yPost "map to Z": ! mapToZ, " toZMap, zToMap Naming > Use the correct grammar
97. How a name in a wrong order is created class UserActionEvent Naming > Use the correct grammar
98. How a name in a wrong order is created class UserActionEvent class UserActionEventSwipe: UserActionEvent() class UserActionEventClick: UserActionEvent() Naming > Use the correct grammar
99. 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
100. How a name in a wrong order is created class UserActionEvent class UserActionEventSwipe: UserActionEvent() class UserActionEventClickMessageText: UserActionEvent() class UserActionEventClickProfileImage: UserActionEvent() Don't focus on comprehensiveness and consistency too much Imagine how it looks on the caller side Naming > Use the correct grammar
101. 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
102. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Describe what rather than who/when
103. 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
104. 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
105. Function name example: Declaration Good ! : Describe what this procedure does class MessageRepository { fun storeReceivedMessage(data: MessageData) { Naming > Describe what rather than who/when
106. 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
107. Function name example: Caller code Good ! : We can know what happens by the calling code repository.storeReceivedMessage(messageData) handler.post { presenter.showNewReceivedMessage(messageData) } Naming > Describe what rather than who/when
108. 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
109. Function name example: Another bad reason The procedure responsibility is ambiguous Naming > Describe what rather than who/when
110. Function name example: Another bad reason The procedure responsibility is ambiguous class MessageViewPresenter { fun onMessageReceived(data: MessageData) { // View presentation code for new message ... Naming > Describe what rather than who/when
111. 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) } // !! Naming > Describe what rather than who/when
112. 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
113. 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
114. 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
115. 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
116. 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
117. 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(...) Causes a bug if 1 and 2 happens at the same time Naming > Describe what rather than who/when
118. 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
119. 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 Naming > Describe what rather than who/when
120. 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
121. 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
122. 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
123. 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
124. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Choose unambiguous words
125. Ambiguous words: Example 1/2 Question: What does initializationFlag represent? - shouldInitialize - isInitializing - is/wasInitialized - isInitializable - isNotInitialized (!!) Naming > Choose unambiguous words
126. Ambiguous words: Example 1/2 Question: What does initializationFlag represent? - shouldInitialize - isInitializing - is/wasInitialized - isInitializable - isNotInitialized (!!) Choose one from the above options (Except for the last one) Naming > Choose unambiguous words
127. Ambiguous words: Example 2/2 Question: What does sizeLimit represent? - max or min? - height, width, byte, characters, or length? Naming > Choose unambiguous words
128. 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
129. Rewording options to consider - flag: is, was, should, can, may, will ... - check: is, query, verify, measure, filter, notify, update ... - good, fine: valid, completed, reliable, secure, satisfies ... - old: previous, stored, expired, invalidated, deprecated ... - tmp, retval: actual name Naming > Choose unambiguous words
130. Rewording options to consider - flag: is, was, should, can, may, will ... - check: is, query, verify, measure, filter, notify, update ... - good, fine: valid, completed, reliable, secure, satisfies ... - old: previous, stored, expired, invalidated, deprecated ... - tmp, retval: actual name Use dictionary and thesaurus Naming > Choose unambiguous words
131. Choose unambiguous words: Summary - Avoid words like flag and check - Use a dictionary and thesaurus Naming > Choose unambiguous words
132. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Avoid confusing abbreviations
133. Abbreviations: Example 1/2 Question: What does im stand for? Naming > Avoid confusing abbreviations
134. Abbreviations: Example 1/2 Question: What does im stand for? input method, illegal message, instance manager ... Naming > Avoid confusing abbreviations
135. 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
136. Abbreviations: Example 2/2 Question: What does str stand for? Naming > Avoid confusing abbreviations
137. Abbreviations: Example 2/2 Question: What does str stand for? string, structure, stream, streak, street, sorted transaction record Naming > Avoid confusing abbreviations
138. 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 fine - str for string is acceptable especially for a limited scope Naming > Avoid confusing abbreviations
139. Project-specific abbreviation Some abbreviations are used project-wide Naming > Avoid confusing abbreviations
140. Project-specific 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
141. Avoid confusing abbreviations: Summary - Don't use your own abbreviations - Commonly used abbreviations are acceptable - Prepare document or glossary for project-specific abbreviations Naming > Avoid confusing abbreviations
142. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Add type/unit suffix
143. 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
144. Add type/unit suffix: Aside Consider creating a wrapper class of a unit Naming > Add type/unit suffix
145. Add type/unit suffix: Aside Consider creating a wrapper class of a unit class Inch(val value: Int) class Centimeter(val value: Int) Naming > Add type/unit suffix
146. Add type/unit suffix: 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 suffix
147. Add type/unit suffix: 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! "value class" (Scala) or "inline class" (Kotlin) may help you Naming > Add type/unit suffix
148. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Use positive affirmation
149. Use positive affirmation ! : Positive words, isEnabled Naming > Use positive affirmation
150. Use positive affirmation ! : Positive words, isEnabled ! : Negative words, isDisabled Naming > Use positive affirmation
151. Use positive affirmation ! : Positive words, isEnabled ! : Negative words, isDisabled ! : Positive words with "not", "no", or "non", isNotEnabled Naming > Use positive affirmation
152. Use positive affirmation ! : 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 affirmation
153. Use positive affirmation ! : Positive words, isEnabled ! : Negative words, isDisabled ! : Positive words with "not", "no", or "non", isNotEnabled ! : Negative words with "not", "no", or "non", isNotDisabled We can rename isNotDisabled → isEnabled, and isNotEnabled → isDisabled without any logic change Naming > Use positive affirmation
154. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation Naming > Adhere to language/platform/project conventions
155. Topics - Use the correct grammar - Describe what rather than who/when - Choose unambiguous words - Avoid confusing abbreviations - Add type/unit suffix - Use positive affirmation - Adhere to language/platform/project conventions Naming > Adhere to language/platform/project conventions
156. Language/platform/project conventions Adhere to convention/rule/manner of language/platform/project Naming > Adhere to language/platform/project conventions
157. 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
158. Summary A name must be accurate, descriptive, and unambiguous Naming > Summary
159. Summary A name must be accurate, descriptive, and unambiguous Pay attention to - describing "what" rather than "who/when" - grammar and word choice Naming > Summary
160. /** Code readability session 3 */ // Comments
161. 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 Comments > Introduction
162. Why we should write comments Readability - Convey intent: summarize, explain reasoning - Prevent mistakes: note precautions - Refactor: rename, simplify Comments > Introduction
163. 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
164. 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
165. 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<String, String>): Boolean Comments > Introduction
166. Refactoring with comments: Refactoring plan Think about why we need a long comment Comments > Introduction
167. 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
168. 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
169. 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
170. Topics - Documentation - Inline comment Comments > Introduction
171. Topics - Documentation - Inline comment Comments > Documentation
172. What we write documentation for - Type: class, struct, interface, protocol, enum ... - Value: field, property, constant, parameter ... - Procedure: global function, method, extension ... - Scope: package, module, namespace ... Comments > Documentation
173. 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
174. Contents of documentation Let's take a look at anti-patterns first Comments > Documentation
175. Anti-patterns: Auto-generated comment /** * @param keyword * @return */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns
176. Anti-patterns: Same to the name /** * Gets the description for a keyword. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns
177. 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
178. Anti-patterns: Referring to private members /** * Returns a string stored in a private map [dictionary]. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns
179. Anti-patterns: No summary /** * Throws an exception if the given `keyword` is empty. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns
180. Anti-patterns: Mentioning to callers /** * ... * This is called by class [UserProfilePresenter]. */ fun getDescription(keyword: String): String Comments > Documentation > Anti-patterns
181. 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
182. 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
183. Contents of documentation Comments > Documentation > Overview
184. Contents of documentation Short summary (Mandatory) "What it is/does" in a phrase/sentence Comments > Documentation > Overview
185. 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
186. 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 > Short summary
187. Short summary: How to write 1/2 Find the most important line/block/element of code Comments > Documentation > Short summary
188. 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
189. 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
190. Short summary: How to write 2/2 Complement information based on the most important point Comments > Documentation > Short summary
191. 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
192. 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
193. 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) The summary will also be a good hint for naming Comments > Documentation > Short summary
194. 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
195. 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 > Details
196. Contents of "details" in documentation - Specification and usage - Function return value - Limitation and error values/status - Examples - ... Comments > Documentation > Details
197. Contents of "details" in documentation - Specification and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details
198. Specification and usage Comment on typical usage and expected behavior Comments > Documentation > Details
199. Specification 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
200. Contents of "details" in documentation - Specification and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details
201. Function return value 1/2 Question: What does the following return value mean? fun setSelectedState(isSelected: Boolean): Boolean Comments > Documentation > Details
202. 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 Comments > Documentation > Details
203. 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
204. 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 isSuccessfullyUpdated // true if there is no error Comments > Documentation > Details
205. 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 isSuccessfullyUpdated // true if there is no error isSelected // pass through the given `isSelected` value ... Comments > Documentation > Details
206. 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
207. 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
208. 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
209. Contents of "details" in documentation - Specification and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details
210. 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
211. 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
212. 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
213. 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
214. Contents of "details" in documentation - Specification and usage - Function return value - Limitation and error values/status - Examples Comments > Documentation > Details
215. 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<String> {... Comments > Documentation > Details
216. Documentation: Summary Comments > Documentation > Summary
217. Documentation: Summary Target: Type, value, procedure, and scope Comments > Documentation > Summary
218. Documentation: Summary Target: Type, value, procedure, and scope Contents: Short summary (mandatory), details (optional) Comments > Documentation > Summary
219. Documentation: Summary Target: Type, value, procedure, and scope Contents: Short summary (mandatory), details (optional) Cautions: - Write only when the name is insufficient as explanation - Don't mention to the caller or the implementation detail Comments > Documentation > Summary
220. Topics - Documentation - Inline comment Comments > Inline comment
221. What inline comment is: Kotlin case Comments > Inline comment
222. 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
223. 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 */ + ... ... - Comment with whatever helps readers - Summary is not mandatory Comments > Inline comment
224. What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment
225. What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment
226. Inline comment for large code Make code blocks with comments to summarize what they do Comments > Inline comment
227. 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
228. 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
229. What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment
230. 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
231. 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) } Question: Why do we need to call reverse()? Comments > Inline comment
232. 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
233. 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) } Prevent from removing reverse() by incorrect "refactoring" Comments > Inline comment
234. What requires inline comments - Large code: Code blocks with comments - Complex/unintuitive code: Summary, reason - Workaround: Background, issue link Comments > Inline comment
235. 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
236. 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 Add links to explain // To avoid Device-X specific tinting bug (see, ISSUE-123456) Comments > Inline comment
237. Inline comments: Summary Essentially the same with documentation - Short summary is not mandatory Comments > Inline comment
238. 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
239. 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
240. const val CODE_READABILITY_SESSION_4 var State
241. 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 State > Introduction
242. Object state and readability 1/2 It's hard to read overly stated code State > Introduction
243. Object state and readability 1/2 It's hard to read overly stated code Question: Reference transparent/immutable == readable? State > Introduction
244. 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
245. Object state and readability 2/2 Let's look at an example of width first search of a binary tree class Node(value: Int, left: Node?, right: Node?) State > Introduction
246. Object state and readability 2/2 Let's look at an example of width first search of a binary tree class Node(value: Int, left: Node?, right: Node?) Case1: With a mutable queue Case2: With recursive call State > Introduction
247. Case1: With a mutable queue fun search(valueToFind: Int, root: Node): Node? { val queue = ArrayDeque<Node>() 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
248. Case2: With recursive call fun search(valueToFind: Int, root: Node): Node? = innerSearch(valueToFind, listOf(root)) tailrec fun innerSearch(valueToFind: Int, queue: List<Node>): 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
249. Problems of the recursive call example State > Introduction
250. 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
251. 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
252. What we should care about State > Introduction
253. 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
254. 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 fine - Fold/recursive call may adversely affect actual state State > Introduction
255. Topics How to simplify states for readability State > Introduction
256. Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > Introduction
257. Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > Orthogonality
258. "Orthogonal" relationship: Definition State > Orthogonality
259. "Orthogonal" relationship: Definition For two properties, they are "orthogonal" if one is modifiable regardless of the other's value State > Orthogonality
260. "Orthogonal" relationship: Example Orthogonal: authorName and layoutVisibility Both properties can be updated regardless of the other value State > Orthogonality
261. "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
262. Why we avoid non-orthogonal relationships Inconsistent state might occur State > Orthogonality
263. Why we avoid non-orthogonal relationships Inconsistent state might occur var coinCount: Int var coinText: String State > Orthogonality
264. 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
265. How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality
266. How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality
267. Replace by using getter/function 1/2 Non-orthogonal properties var coinCount: Int var coinText: String State > Orthogonality > Remove with getter/function
268. 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
269. Replace by using getter/function 2/2 Create coinText value by coinCount var coinCount: Int var coinText: String State > Orthogonality > Remove with getter/function
270. 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
271. Replace by using getter/function 2/2 Create coinText value by coinCount var coinCount: Int val coinText: String get() = resource.getQuantityString(..., coinCount) The modifiable property is only coinCount State > Orthogonality > Remove with getter/function
272. How to remove non-orthogonal relationships - Replace with a property getter or function - Encapsulate by algebraic data type State > Orthogonality
273. 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
274. 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
275. 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
276. Algebraic data type Data type to represent direct sum State > Orthogonality > Encapsulate with an algebraic class
277. 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
278. 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
279. 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
280. 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
281. 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
282. 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
283. 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
284. Emulate algebraic data type 1/2 Some language does not have algebraic data type (e.g., Java) State > Orthogonality > Encapsulate with an algebraic class
285. 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
286. 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
287. Emulate algebraic data type 2/2 class QueryResponse { @Nullable private final String resultText; @Nullable private final Integer errorCode; private QueryResponse(...) { ... } State > Orthogonality > Encapsulate with an algebraic class
288. 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
289. 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
290. 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
291. 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
292. 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
293. "Orthogonal" relationship: Summary State > Orthogonality > Summary
294. "Orthogonal" relationship: Summary Orthogonal relationship - Updatable independently - Good to remove invalid state State > Orthogonality > Summary
295. "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
296. Topics How to simplify states for readability - "Orthogonal" relationship - State design strategies State > State design strategy
297. Types of object state transitions State > State design strategy
298. 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
299. 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
300. Immutable object All properties won't be changed State > State design strategy > Immutable
301. 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
302. 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
303. Immutable properties Make properties immutable if possible State > State design strategy > Immutable
304. Immutable properties Make properties immutable if possible class ItemListCategoryData( // Use val if no need to update var itemCategoryName: String, State > State design strategy > Immutable
305. 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<ItemModel> ) State > State design strategy > Immutable
306. 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 > Idempotent
307. Idempotency The result of multiple operations is the same as single operation State > State design strategy > Idempotent
308. 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
309. 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 The side-effect may be hidden with a wrapper (e.g., Lazy) State > State design strategy > Idempotent
310. State graph of idempotent object There are two states and one direction path ┌─────┐ ▼ │ ┌──────┐ ┌──────┐ │ │ Open │──────▶│Closed│──┘ └──────┘ └──────┘ State > State design strategy > Idempotent
311. Why idempotency is good Encapsulate internal state to call a function State > State design strategy > Idempotent
312. 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
313. Why idempotency is good Encapsulate internal state to call a function // We may forget to check `isClosed` if (!nonIdempotentClosable.isClosed()) { nonIdempotentClosable.close() } // We can simply call `close` for an idempotent instance idempotentClosable.close() State > State design strategy > Idempotent
314. Non-idempotent case It's NOT idempotent if the first state can remain after the operation State > State design strategy > Idempotent
315. Non-idempotent case It's NOT idempotent if the first state can remain after the operation fun get(): Result? { if (latestResult == null) { latestResult = queryToServer() // May return null } return latestResult State > State design strategy > Idempotent
316. Non-idempotent case It's NOT idempotent if the first state can remain after the operation fun get(): Result? { if (latestResult == null) { latestResult = queryToServer() // May return null } return latestResult The return values of the first call and the second can differ get() != get() // This may be true!! State > State design strategy > Idempotent
317. Non-idempotent state transition graph The first state also has a cycle to itself ┌─────┐ ┌─────┐ ▼ │ ▼ │ ┌──────┐ │ ┌──────┐ │ │ Null │──┴───▶│Result│──┘ └──────┘ └──────┘ State > State design strategy > Idempotent
318. Non-idempotent state transition graph The first state also has a cycle to itself ┌─────┐ ┌─────┐ ▼ │ ▼ │ ┌──────┐ │ ┌──────┐ │ │ Null │──┴───▶│Result│──┘ └──────┘ └──────┘ The side-effect should not be hidden State > State design strategy > Idempotent
319. 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 > Acyclic
320. Acyclic state Instance only used for a specific argument - Another instance is created for another argument - The performance is not good sometimes State > State design strategy > Acyclic
321. Acyclic state Instance only used for a specific 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
322. 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
323. Why acyclic state is good 1/2 Let's compare with a type with cyclic states State > State design strategy > Acyclic
324. 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
325. Cyclic state transition graph Loopback to "Loading" with a new video path ┌───────────────────────────────┐ ▼ │ ┌────────┐ ┌────────┐ ┌────────┐ │ │Loading │─▶│Playing │──▶│Finished│──┤ └────────┘ └────────┘ └────────┘ │ │ │ ┌────────┐ │ └───────────┴──────▶│ Error │──┘ └────────┘ State > State design strategy > Acyclic
326. Why acyclic state is good 2/2 Need to check the current loop argument if there's a cycle State > State design strategy > Acyclic
327. 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
328. 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
329. 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() // Not thread safe still State > State design strategy > Acyclic
330. 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
331. 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
332. 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
333. 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 > Summary
334. State design strategy: Summary - Make properties immutable - Idempotency is also good - Remove/minimize state cycle State > State design strategy > Summary
335. Summary Minimize state for readability and robustness - Don't make it an objective State > Summary
336. 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
337. 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 Care about state design strategy - Immutability, idempotency and cycle State > Summary
338. abstract fun codeReadabilitySession5() fun Procedure()
339. 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 Procedure > Introduction
340. What "procedure" is - main/subroutine - function - method - computed property - property getter/setter - constructor/initialization block - ... Procedure > Introduction
341. What a readable procedure is Procedure > Introduction
342. What a readable procedure is Predictable - Consistent with the name - Easy to write documentation - Few error cases or limitations Procedure > Introduction
343. Topics Make procedure responsibility/flow clear Procedure > Introduction
344. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Introduction
345. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Responsibility > Check with a short summary
346. Split procedure if it's hard to summarize Try to summarize what the procedure does Procedure > Responsibility > Check with a short summary
347. Procedure summary: Example 1/2 messageView.text = messageData.contentText senderNameView.text = messageData.senderName timestampView.text = messageData.sentTimeText Procedure > Responsibility > Check with a short summary
348. 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
349. Procedure summary: Example 2/2 messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } Procedure > Responsibility > Check with a short summary
350. Procedure summary: Example 2/2 messageView.text = messageData.contentText doOnTransaction { messageDatabase.insertNewMessage(messageData) } How to summarize this Procedure > Responsibility > Check with a short summary
351. 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
352. Procedure responsibility and summary Hard to summarize = Typical bad signal - Split the procedure into sub-procedures Procedure > Responsibility > Check with a short summary
353. 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
354. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Responsibility > Return value and side-effect
355. 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
356. 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 modification 7 Eiffel: a language for software engineering, Meyer Bertrand, 2012 6 https://martinfowler.com/bliki/CommandQuerySeparation.html Procedure > Responsibility > Return value and side-effect
357. Command-query separation: Example 1/2 class IntList(vararg elements: Int) { infix fun append(others: IntList): IntList = ... } Procedure > Responsibility > Return value and side-effect
358. 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
359. 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
360. 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
361. 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
362. 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
363. 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
364. Command-query separation: Drawbacks 2/3 Bad code example: class UserDataStore { // Command fun saveUserData(userData: UserData) = ... } Procedure > Responsibility > Return value and side-effect
365. 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
366. 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
367. 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
368. 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
369. Responsibility on return value and side-effect Don't modify when returning a "main" result Procedure > Responsibility > Return value and side-effect
370. Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modification Procedure > Responsibility > Return value and side-effect
371. Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modification - Main result: conversion/calculation result, a new instance ... - Sub result: error type, metadata of modification (stored size) ... (Documentation is mandatory) Procedure > Responsibility > Return value and side-effect
372. Responsibility on return value and side-effect Don't modify when returning a "main" result = Acceptable to return a "sub" result with modification - Main result: conversion/calculation result, a new instance ... - Sub result: error type, metadata of modification (stored size) ... (Documentation is mandatory) Acceptable if the interface is a de facto standard e.g., fun Queue<T>.poll(): T Procedure > Responsibility > Return value and side-effect
373. Make responsibility clear: Summary Procedure > Responsibility > Summary
374. Make responsibility clear: Summary Split a procedure if it's hard to summarize - Try to write documentation anyway Procedure > Responsibility > Summary
375. 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 modification - Fine if the interface is a de facto standard Procedure > Responsibility > Summary
376. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Flow
377. How to find if procedure flow is clear Procedure > Flow
378. How to find if procedure flow is clear Try to write short summary Procedure > Flow
379. How to find if procedure flow is clear Try to write short summary - Hard to write when the flow is unclear Procedure > Flow
380. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Flow > Definition-based programming
381. Definition-based programming Prefer to define value/procedure with a name Procedure > Flow > Definition-based programming
382. 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
383. Definition-based programming: Bad example Procedure > Flow > Definition-based programming
384. Definition-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 > Definition-based programming
385. What is wrong with the bad example Hard to make summary of the procedure Procedure > Flow > Definition-based programming
386. 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 > Definition-based programming
387. 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
388. 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
389. 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
390. 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
391. 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
392. 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
393. 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
394. Definition-based programming: Pitfall Extracting may make the code less readable Procedure > Flow > Definition-based programming
395. Definition-based programming: Pitfall Extracting may make the code less readable - Unnecessary state - Unnecessary strong coupling (will explain at the next session) Procedure > Flow > Definition-based programming
396. 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 > Definition-based programming
397. 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 > Definition-based programming
398. Pitfalls of extraction: What is wrong - Unnecessary mutability and nullability - Unsafe to call initialize... methods multiple times - Function name initialize... is ambiguous Procedure > Flow > Definition-based programming
399. Pitfalls of extraction: What is wrong - Unnecessary mutability and nullability - Unsafe to call initialize... methods multiple times - Function name initialize... is ambiguous Optimize the scope of the extracted methods Procedure > Flow > Definition-based programming
400. Pitfalls of extraction: Good example Extract view creation and initialization procedure Procedure > Flow > Definition-based programming
401. 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 > Definition-based programming
402. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Flow > Focus on normal cases
403. 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
404. Normal cases and error cases: Example String.toIntOrNull() Procedure > Flow > Focus on normal cases
405. 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
406. 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
407. Procedure flow and normal/error cases Procedure > Flow > Focus on normal cases
408. Procedure flow and normal/error cases Filter error cases to focus on the normal case Procedure > Flow > Focus on normal cases
409. Procedure flow and normal/error cases Filter error cases to focus on the normal case Apply early return Procedure > Flow > Focus on normal cases
410. Early return: Bad example Procedure > Flow > Focus on normal cases
411. 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
412. Early return: What is wrong Unclear Procedure > Flow > Focus on normal cases
413. 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
414. Early return: Good example Procedure > Flow > Focus on normal cases
415. 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
416. 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
417. Topics - Make responsibility clear - Split if it's hard to summarize - Split if it has both return value and side-effect - Make flow clear - Perform definition-based programming - Focus on normal cases - Split by object, not condition Procedure > Flow > Split by object
418. 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
419. 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
420. 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] Two ways to implement this matrix Procedure > Flow > Split by object
421. Bad example of "split by condition first" fun bindViewData(accountType: AccountType) { when (accountType) { PREMIUM -> updateViewsForPremium() FREE -> updateViewsForFree() } } Procedure > Flow > Split by object
422. Bad example of "split by condition first" fun bindViewData(accountType: AccountType) { when (accountType) { PREMIUM -> updateViewsForPremium() FREE -> updateViewsForFree() } } private fun updateViewsForPremium() { backgroundView.color = PREMIUM_BACKGROUND_COLOR accountTypeIcon.image = resources.getImage(PREMIUM_IMAGE_ID) } Procedure > Flow > Split by object
423. 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
424. 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
425. 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 Make supporting function or code block for each target object first Procedure > Flow > Split by object
426. Split by object: Good example 1/2 fun bindViewData(accountType: AccountType) { updateBackgroundViewColor(accountType) updateAccountTypeImage(accountType) } Procedure > Flow > Split by object
427. 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
428. 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
429. 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
430. 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 - Can conduct further more refactoring e.g., Make extracted functions reference transparent Procedure > Flow > Split by object
431. 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
432. Early return VS split by object Doesn't "early return" represent "split by condition"? Procedure > Flow > Split by object
433. 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
434. 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
435. Procedure flow: Summary Procedure > Flow > Summary
436. Procedure flow: Summary Perform definition-based programming: Extract nests/chains to a named value/function Procedure > Flow > Summary
437. Procedure flow: Summary Perform definition-based programming: Extract nests/chains to a named value/function Focus on normal cases: Apply early return Procedure > Flow > Summary
438. Procedure flow: Summary Perform definition-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
439. Summary Check whether it's easy to write a short summary Procedure > Summary
440. 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
441. 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 flow: - Apply definition based programing, early return, split by object Procedure > Summary
442. Code→readability→session 6 Dependency ‐
443. 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 Dependency
444. 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
445. 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
446. What "dependency" is Is the word "dependency" only for class? Dependency
447. What "dependency" is Is the word "dependency" only for class? No - Procedure, module, instance ... Dependency
448. Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency
449. Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Coupling
450. Coupling A property to represent how strong the dependency is ▲ Strong │ │ │ │ │ │ ▼ Weak Dependency > Coupling
451. 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
452. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling
453. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Content coupling
454. Content coupling Relies on the internal workings Dependency > Coupling > Content coupling
455. Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling
456. Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling
457. Allows illegal usage: Bad example class Caller { fun callCalculator() { calculator.calculate() ... Dependency > Coupling > Content coupling
458. Allows illegal usage: Bad example class Caller { fun callCalculator() { calculator.parameter = 42 calculator.calculate() val result = calculator.result ... Dependency > Coupling > Content coupling
459. 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
460. Allows illegal usage: Bad points Dependency > Coupling > Content coupling
461. 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
462. 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. Not robust to wrong usage and implementation change Dependency > Coupling > Content coupling
463. Allows illegal usage: How to fix - Encapsulate stateful call sequence - Pass/receive data as arguments or a return value Dependency > Coupling > Content coupling
464. Allows illegal usage: Fixed code example class Caller { fun callCalculator() { val result = calculator.calculate(42) ... Dependency > Coupling > Content coupling
465. 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
466. Allows illegal usage: What to check Dependency > Coupling > Content coupling
467. Allows illegal usage: What to check Don't apply design concept/principal too much - e.g., command/query separation Dependency > Coupling > Content coupling
468. Content coupling Relies on the internal workings Example: - Allows illegal usage - Shares internal mutable state Dependency > Coupling > Content coupling
469. Shares mutable state: Bad example class UserListPresenter(val userList: List<UserData>) { fun refreshViews() { /* ... */ } Dependency > Coupling > Content coupling
470. Shares mutable state: Bad example class UserListPresenter(val userList: List<UserData>) { fun refreshViews() { /* ... */ } class Caller { val userList: List<UserData> = mutableListOf() val presenter = UserListPresenter(userList) ... Dependency > Coupling > Content coupling
471. Shares mutable state: Bad example class UserListPresenter(val userList: List<UserData>) { fun refreshViews() { /* ... */ } class Caller { val userList: List<UserData> = mutableListOf() val presenter = UserListPresenter(userList) ... fun addUser(newUser: UserData) { userList += newUser presenter.refreshViews() Dependency > Coupling > Content coupling
472. 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 Dependency > Coupling > Content coupling
473. 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 find the root cause - A reference to the presenter is not required to update it Dependency > Coupling > Content coupling
474. Shares mutable state: How to fix - Make values immutable, copy arguments, or apply copy-on-write - Limit interfaces to update the state Dependency > Coupling > Content coupling
475. Shares mutable state: Fixed code example class UserListPresenter { val userList: MutableList<UserData> = mutableListOf() Dependency > Coupling > Content coupling
476. Shares mutable state: Fixed code example class UserListPresenter { val userList: MutableList<UserData> = mutableListOf() fun addUsers(newUsers: List<UserData>) { userList += newUsers // Update views with `userList` } Dependency > Coupling > Content coupling
477. Shares mutable state: Fixed code example class UserListPresenter { val userList: MutableList<UserData> = mutableListOf() fun addUsers(newUsers: List<UserData>) { userList += newUsers // Update views with `userList` } No need to worry about whether newUsers is mutable or not (if addUsers is atomic) Dependency > Coupling > Content coupling
478. Shares mutable state: What to check Dependency > Coupling > Content coupling
479. Shares mutable state: What to check Make a mutable value ownership clear Dependency > Coupling > Content coupling
480. 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
481. Content coupling: Summary Don't rely on internal working - Remove illegal usage - Make mutable data ownership clear Dependency > Coupling > Content coupling
482. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Common coupling
483. Common coupling Shares (mutable) global data Dependency > Coupling > Common coupling
484. Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use files / databases / shared resources) Dependency > Coupling > Common coupling
485. Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use files / databases / shared resources) Dependency > Coupling > Common coupling
486. Pass data by global variables: Bad example var parameter: Int? = null var result: Data? = null Dependency > Coupling > Common coupling
487. 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
488. 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
489. 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
490. Pass data by global variables: How to fix Pass as a function parameter or return value Dependency > Coupling > Common coupling
491. Pass data by global variables: Fixed code class Calculator { fun calculate(parameter: Int): Int = parameter + ... Dependency > Coupling > Common coupling
492. 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
493. Common coupling Shares (mutable) global data - Pass data by global variables - Depend on a mutable singleton - (Use files / databases / shared resources) Dependency > Coupling > Common coupling
494. Depend on a mutable singleton: Bad example val USER_DATA_REPOSITORY = UserDataRepository() Dependency > Coupling > Common coupling
495. Depend on a mutable singleton: Bad example val USER_DATA_REPOSITORY = UserDataRepository() class UserListUseCase { suspend fun invoke(): List<User> = withContext(...) { val result = USER_DATA_REPOSITORY.query(...) ... Dependency > Coupling > Common coupling
496. Mutable global variables: Bad points Singleton is not managed - Unmanaged singleton lifecycle, scope, and references - Not robust for specification change - Bad testability Dependency > Coupling > Common coupling
497. Mutable global variables: How to fix Pass an instance as a constructor parameter (= Dependency injection) The constructor caller can manage the instance Dependency > Coupling > Common coupling
498. Mutable global variables: Fixed code class UserListUseCase( val userDataRepository: UserDataRepository ) { suspend fun invoke(): List<User> = withContext(...) { val result = userDataRepository.query(...) ... Dependency > Coupling > Common coupling
499. Common coupling: Note Apply the same discussion to a smaller scope - module, package, type ... Example: Use a return value instead of instance field Dependency > Coupling > Common coupling
500. 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
501. 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
502. Common coupling: Summary No more singletons nor global mutable values - Unmanageable, not robust, hard to test ... Dependency > Coupling > Common coupling
503. 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
504. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Control coupling
505. Control coupling Switches logic by passing flag "what to do" - Has large conditional branch covering procedure - Each branch body is unrelated to the other branches Dependency > Coupling > Control coupling
506. Control coupling example: Boolean flag 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
507. 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
508. Control coupling: Bad points Unreadable - Hard to summarize what it does - Difficult to name and comment Dependency > Coupling > Control coupling
509. Control coupling: Bad points Unreadable - Hard to summarize what it does - Difficult to name and comment Not robust - Requires the similar conditional branch to the callers - Fragile against condition update Dependency > Coupling > Control coupling
510. Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling
511. Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling
512. Remove conditional branch Split procedures for each argument - Remove parameter from each new procedure Dependency > Coupling > Control coupling
513. 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
514. 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
515. 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
516. Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling
517. Split by object Create code block for each target object, not condition - Break down content coupling into smaller scope Dependency > Coupling > Control coupling
518. 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
519. 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
520. 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
521. Ideas for improvement - Remove conditional branch - Split by object (introduced last session) - Apply strategy pattern Dependency > Coupling > Control coupling
522. 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
523. 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
524. 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
525. 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
526. Control coupling: Summary Don't create large conditional branch covering a whole procedure Dependency > Coupling > Control coupling
527. 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
528. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Stamp coupling
529. Stamp coupling Passes a structure though some of the data that are not used Dependency > Coupling > Stamp coupling
530. 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
531. Stamp coupling: Example updateUserView(userData) fun updateUserView(userData: UserData) { Dependency > Coupling > Stamp coupling
532. 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
533. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Data coupling
534. 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
535. Coupling Bad/strong coupling: content, common, control Fine/weak coupling: stamp, data, message Dependency > Coupling > Message coupling
536. Message coupling Call a method without any data - Hardly happens - May cause content coupling in another layer Dependency > Coupling > Message coupling
537. 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
538. Coupling: Summary Avoid strong coupling - Use stamp/data coupling instead of content/common/control Dependency > Coupling > Summary
539. Coupling: Summary Avoid strong coupling - Use stamp/data coupling instead of content/common/control Content coupling: Relies on internal working Dependency > Coupling > Summary
540. 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
541. 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 Control coupling: Has large conditional branch Dependency > Coupling > Summary
542. Code←readability←session 7 Dependency ‐‐
543. 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 Dependency
544. Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Direction
545. Direction Keep the dependency in one direction as far as possible = It’s good if there’s no cycle Dependency > Direction
546. 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
547. Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction
548. Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction
549. Bad code example class MediaViewPresenter { fun getVideoUri(): Uri = ... fun playVideo() { videoPlayerView.play(this) ... Dependency > Direction > Caller to callee
550. 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
551. What is to be checked Call stack of the example - MediaViewPresenter.playVideo() - VideoPlayerView.play(...) - MediaViewPresenter.getVideoUri() Dependency > Direction > Caller to callee
552. 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
553. How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee
554. How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee
555. 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
556. 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
557. How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee
558. Extract small classes Can't pass value directly for asynchronous evaluation Dependency > Direction > Caller to callee
559. Extract small classes Can't pass value directly for asynchronous evaluation ┌────────────────────────┐ │ MediaViewPresenter │ │ │───────▶┌──────────────┐ │ getVideoUri() ◀──────┼────────│ VideoPlayer │ │ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee
560. Extract small classes Can't pass value directly for asynchronous evaluation ┌────────────────────────┐ │ MediaViewPresenter │ │ │───────▶┌──────────────┐ │ getVideoUri() ◀──────┼────────│ VideoPlayer │ │ │ └──────────────┘ └────────────────────────┘ Remove callback by extracting depended logic Dependency > Direction > Caller to callee
561. Extract small classes: Step 1 Create a named class for the callback ┌────────────────────────┐ │ MediaViewPresenter │ │ │───────▶┌──────────────┐ │ getVideoUri() ◀──────┼────────│ VideoPlayer │ │ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee
562. Extract small classes: Step 1 Create a named class for the callback ┌────────────────────────┐ │ MediaViewPresenter │ │ ┌──────────────────┐ │───────▶┌──────────────┐ │ │ VideoUriProvider │◀──┼────────│ VideoPlayer │ │ └──────────────────┘ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee
563. Extract small classes: Step 2 Move the named class out of the outer class ┌────────────────────────┐ │ MediaViewPresenter │ │ ┌──────────────────┐ │───────▶┌──────────────┐ │ │ VideoUriProvider │◀──┼────────│ VideoPlayer │ │ └──────────────────┘ │ └──────────────┘ └────────────────────────┘ Dependency > Direction > Caller to callee
564. Extract small classes: Step 2 Move the named class out of the outer class ┌────────────────────┐ │ MediaViewPresenter │─────┐ └────────────────────┘ │ ┌──────────────┐ │ └─────▶│ VideoPlayer │ │ └──────────────┘ ▼ │ ┌──────────────────┐ │ │ VideoUriProvider │◀────────────────────┘ └──────────────────┘ Dependency > Direction > Caller to callee
565. How to remove reference to caller Remove unnecessary callback - Pass values as parameters - Extract small classes Dependency > Direction > Caller to callee
566. Dependency on caller: Exceptions Dependency on a caller may be acceptable for the following: Dependency > Direction > Caller to callee
567. 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
568. 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
569. Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Detail to abstraction
570. Dependency on detail Don't depend on inheritances = Don't check type of this or self Exception: Sealed class Dependency > Direction > Detail to abstraction
571. Bad code example open class IntList { fun addElement(value: Int) { if (this is ArrayIntList) { ... } else { ... ... class ArrayIntList: IntList() { ... Dependency > Direction > Detail to abstraction
572. What's wrong with the bad example Brakes encapsulation and not robust - For inheritance specification change - For adding a new inheritance Dependency > Direction > Detail to abstraction
573. 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
574. Dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Complex to simple
575. Bad code example class UserData( val userId: UserId, ... ) class UserDataRequester { fun obtainFromServer(): UserData { ... Dependency > Direction > Complex to simple
576. Bad code example class UserData( val userId: UserId, ... val requester: UserDataRequester ) class UserDataRequester { fun obtainFromServer(): UserData { ... Dependency > Direction > Complex to simple
577. 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
578. 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
579. 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
580. Exceptions It's sometimes necessary to depend on complex types - Mediator pattern - Adapter or facade - Data binder Dependency > Direction > Complex to simple
581. Preferred dependency direction - Caller to callee - Detail to abstraction - Complex to simple Dependency > Direction > Summary
582. 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
583. Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Redundancy
584. Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy
585. Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy
586. Cascaded dependency Avoid unnecessary dependency chain Dependency > Redundancy > Cascaded dependency
587. Cascaded dependency Avoid unnecessary dependency chain ┌──────────────────────┐ │ MessageTextPresenter │ └──────────────────────┘ │ has instance ▼ ┌──────────────────────┐ │ TimestampPresenter │ └──────────────────────┘ │ has instance ▼ ┌──────────────────────┐ │ MessageDataProvider │ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency
588. Cascaded dependency Avoid unnecessary dependency chain ┌──────────────────────┐ depending indirectly │ MessageTextPresenter │─ ─ ─ ─ ─ ─ ─ ─ ┐ └──────────────────────┘ │ has instance │ ▼ (only for MessageDataProvider) ┌──────────────────────┐ │ │ TimestampPresenter │ └──────────────────────┘ │ │ has instance ▼ │ ┌──────────────────────┐ │ MessageDataProvider │◀ ─ ─ ─ ─ ─ ─ ─ ┘ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency
589. Bad cascaded dependency example class TimestampPresenter { val dataProvider: MessageDataProvider = ... fun invalidateViews() { // Update timestamp by `dataProvider` ... Dependency > Redundancy > Cascaded dependency
590. 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
591. 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
592. How to fix 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
593. How to fix cascaded dependency Flatten nested dependencies by means of direct dependencies ┌──────────────────────┐ │ MessageTextPresenter │──┐ └──────────────────────┘ │ │ has instance ┌──────────────────────┐ ├──────────────▶│ MessageDataProvider │ ┌──────────────────────┐ │ └──────────────────────┘ │ TimestampPresenter │──┘ └──────────────────────┘ Dependency > Redundancy > Cascaded dependency
594. 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
595. Redundant dependency Cascaded dependency - Nested & indirect dependency Redundant dependency set - N:M dependency Dependency > Redundancy > Dependency set
596. Redundant dependency set Avoid duplication of "depending class set" ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ ├───────────────────────────┘ │ │ ▼ ┌─────────────────────────┐ │ MessageDataProvider │ └─────────────────────────┘ Dependency > Redundancy > Dependency set
597. Redundant dependency set Avoid duplication of "depending class set" ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ │ └ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┼ ─ ─ ┤ ├───────────────────────────┘ │ │ │ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set
598. What's wrong with redundant dependency set? Fragile for modification - When a new depending/depended type is added Duplicates switching logic - Provider selection/fallback logic Dependency > Redundancy > Dependency set
599. How to remove redundant dependency set Create an intermediate layer to consolidate dependency set ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ │ │ │ │ │ │ │ └ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┼ ─ ─ ┤ ├───────────────────────────┘ │ │ │ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set
600. How to remove redundant dependency set Create an intermediate layer to consolidate dependency set ┌─────────────────────────┐ ┌─────────────────────────┐ │ MessageTextPresenter │ │ TimestampPresenter │ └─────────────────────────┘ └─────────────────────────┘ └────────────────┬────────────────┘ ▼ ┌─────────────────────────┐ │ MessageDataProvider │ └─────────────────────────┘ ┌────────────────┴────────────────┐ ▼ ▼ ┌─────────────────────────┐ ┌─────────────────────────┐ │LocalMessageDataProvider │ │ServerMessageDataProvider│ └─────────────────────────┘ └─────────────────────────┘ Dependency > Redundancy > Dependency set
601. 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
602. Redundancy: Summary - Remove dependencies only for providing another dependency - Add layer to remove dependency set duplication Dependency > Redundancy > Summary
603. Topics First session: - Coupling Second session: - Direction - Redundancy - Explicitness Dependency > Explicitness
604. Implicit dependency A dependency does not appear on a class diagram Dependency > Explicitness
605. Implicit dependency A dependency does not appear on a class diagram Avoid implicit dependencies, use explicit ones instead Dependency > Explicitness
606. Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness
607. Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness > Implicit data domain
608. 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
609. Why implicit data domain is bad Hard to find which value is accepted - "blue", "RED", "FF0000" are invalid Dependency > Explicitness > Implicit data domain
610. Why implicit data domain is bad Hard to find 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
611. 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
612. 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
613. Other options instead of defining 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
614. Other options instead of defining 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
615. Other options instead of defining 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 Note 2: An "error value" may also require another data domain type Dependency > Explicitness > Implicit data domain
616. Implicit dependency example - Implicit data domain for parameters - Implicitly expected behavior of subtype Dependency > Explicitness > Subtype behavior
617. Implicit dependency by subtype A caller depends on the inherited type implicitly ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit │ │ │ │ ┌───────────────────────┐ │ListenerImplementation │ └───────────────────────┘ Dependency > Explicitness > Subtype behavior
618. Implicit dependency by subtype A caller depends on the inherited type implicitly ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit ┌─────┼───────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ListenerImplementation │ │ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior
619. Implicit dependency example by subtype interface ListenerInterface { fun queryInt(): Int } class LargeClass { inner class ListenerImplementation : ListenerInterface { ... } } class Caller(listener : ListenerInterface) { ... } Dependency > Explicitness > Subtype behavior
620. What's wrong with subtype Depends on result of the function call on LargeClass implicitly - Does not appear on class diagram - Hard to find Caller relying on LargeClass implementation detail Dependency > Explicitness > Subtype behavior
621. How to remove implicit dependency Remove the interface if there is only one implementation ┌───────────────────────┐ call ┌───────────────────────┐ │ ListenerInterface │◀─────────│ Caller │ └───────────────────────┘ └───────────────────────┘ ▲ │ inherit ┌─────┼───────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ListenerImplementation │ │ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior
622. How to remove implicit dependency Remove the interface if there is only one implementation call ┌───────────────────────┐ ┌─────│ Caller │ │ └───────────────────────┘ │ │ ┌─────────────────────────────┐ │ │ LargeClass │ │ │ ▲ │ │ │ │ │ │ │ ┌───────────────────────┐ │ │ │ │ Listener │◀─┼─┘ │ └───────────────────────┘ │ └─────────────────────────────┘ Dependency > Explicitness > Subtype behavior
623. Fixed code example class LargeClass { inner class Listener { fun ... } } class Caller(listener : Listener) { ... } Dependency > Explicitness > Subtype behavior
624. Explicitness: Summary Make dependencies visible on a class diagram - Define types to represent data domain - Remove unnecessary interface Dependency > Explicitness > Summary
625. 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
626. Code-Readability / Session-8 Review #1
627. 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 Review > Introduction
628. Why do we need code review? Review > Introduction
629. Why do we need code review? Most important: Readability - Approve code change only when it is clean enough Review > Introduction
630. 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
631. Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Introduction
632. Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Create PR
633. How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR
634. How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR
635. 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
636. 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
637. Unnecessary commits Remove or squash "reverting commit" Review > Create PR > Commit structure > Squash unnecessary commits
638. 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
639. How to remove unnecessary commits Use git rebase -i or similar command Review > Create PR > Commit structure > Squash unnecessary commits
640. 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
641. 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
642. 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
643. 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
644. 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
645. Example of splitting PR class Deprecated { fun functionA() { /* ... */ } fun functionB() { /* ... */ } } class New { /* ... */ } Review > Create PR > Commit structure > Cut of axis
646. 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
647. 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
648. 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
649. Better option for axis Review > Create PR > Commit structure > Cut of axis
650. Better option for axis Option B is better than A Review > Create PR > Commit structure > Cut of axis
651. Better option for axis Option B is better than A Reason: - Easy to understand the intention - Verifiable that there is no logical change - Traceable commit history Review > Create PR > Commit structure > Cut of axis
652. How to create PR - Tell your intention with commit structure - Keep your PR small Review > Create PR > Keep PR small
653. 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
654. 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
655. 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
656. Top-down approach Fill implementation detail later Review > Create PR > Keep PR small > Split daily work
657. 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
658. 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
659. Bottom-up approach Create small parts first Review > Create PR > Keep PR small > Split daily work
660. Bottom-up approach Create small parts first 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
661. Bottom-up approach Create small parts first 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
662. 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
663. 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
664. 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
665. 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
666. 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
667. Best way to create a PR 2/3 2: Move Commit2_Old to the first 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
668. 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
669. 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
670. Conflict between author and reviewer Author's responsibility: - Keep the PR small Review > Create PR > Keep PR small > Specify scope of PR
671. Conflict 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
672. Conflict 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 conflict with each other Review > Create PR > Keep PR small > Specify scope of PR
673. 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
674. 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
675. Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Address review comment
676. Important point in addressing comments Review > Address review comment
677. Important point in addressing comments Don't just address comments Review > Address review comment
678. 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
679. 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
680. Think why reviewer asked/misunderstood Typical signal that the code is unreadable or wrong Review > Address review comment
681. 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
682. 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
683. Understand reviewer's suggestion Don't just paste attached code in a review comment - Think what is the key idea - Consider renaming - Confirm it's logically correct: exceptions, race conditions ... Review > Address review comment
684. 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
685. Consider addressing a comment to other parts Find similar code and address it Review > Address review comment
686. 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
687. 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
688. 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
689. 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
690. Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Reviewer principles
691. Principles for reviewers Don't be too kind Review > Reviewer principles
692. Principles for reviewers Don't be too kind (but be kind a little) Review > Reviewer principles
693. 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
694. 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
695. Don't neglect review requests Define 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
696. Reactions to review requests GOOD: NOT SO GOOD: - Simply ignore WORST: Review > Reviewer principles > Don't neglect
697. Reactions to review requests GOOD: NOT SO GOOD: - Simply ignore WORST: - Reply "I'll review" without review Review > Reviewer principles > Don't neglect
698. 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
699. When you cannot review If you are busy, reply so Review > Reviewer principles > Don't neglect
700. 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
701. 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
702. Reject problematic PR Don't spend too much time reviewing a problematic PR Review > Reviewer principles > Reject problematic PR
703. Reject problematic PR Ask the author to re-create a PR without reading the detail Review > Reviewer principles > Reject problematic PR
704. 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
705. 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
706. 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 guess author's situation
707. Don't guess author's situation Reviewers should not care about deadline - Let them explain if they want to merge soon - Need to file issue tickets, and add TODO at least Review > Reviewer principles > Don't guess author's situation
708. Don't guess author's situation Reviewers should not care about deadline - Let them explain if they want to merge soon - Need to file issue tickets, and add TODO at least Exception: Major/critical issues on a release branch or hot-fix - Tests are still required - Reviewers should keep cooler than authors Review > Reviewer principles > Don't guess author's situation
709. 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 > Review comment options
710. Options of a review comment The answer is not only showing answer Review > Reviewer principles > Review comment options
711. 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
712. 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
713. 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
714. Topics Authors: - How to create PR - How to address review comment Reviewers: - Principles - Comment contents Review > Reviewer principles
715. 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
716. 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 That's all! Review > Comment contents
717. Code review case study Adding a parameter to function Review > Comment contents > Case study
718. Code review case study Adding a parameter to function fun showPhotoView( photoData: PhotoData ) { if (!photoData.isValid) { return } ... Review > Comment contents > Case study
719. 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
720. Code review case study Naming: Describe what it is rather than how it is used isFromEditor -> showsDialogOnError Review > Comment contents > Case study
721. 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
722. 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
723. Code review case study Dependency: Control coupling with a boolean flag Extract showDialog call Review > Comment contents > Case study
724. Code review case study Dependency: Control coupling with a boolean flag Extract showDialog call - For editor: val isViewShown = showPhotoView(...) if (!isViewShown) { showErrorDialog(...) } - For other place: showPhotoView(...) Review > Comment contents > Case study
725. Code review case study Dependency: Control coupling with a boolean flag fun showPhotoView(photoData: PhotoData): Boolean { if (!photoData.isValid) { return false } ... Review > Comment contents > Case study
726. 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
727. 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
728. What we comment: Summary Review the following items - Styles, conventions, manners, idioms ... - Principles, natural language, structure, dependency ... Review > Comment contents > Summary
729. 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
730. Summary Review is for readability Review > Summary
731. Summary Review is for readability Author: - Keep PR small and structured - Understand review comments, don't just address Review > Summary
732. 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