Refactoring Risky Code

How do you refactor Bluetooth code on Android with zero test coverage? When touching such code you have to test it with real phones and Bluetooth devices to ensure you are not breaking anything. Thus changes come with a high risk of introducing new bugs. At mySugr we spent quite some time refactoring our Bluetooth code, so that we can write plain Kotlin unit tests for it and easily add new functionality.

You are going to see how we achieved the refactoring with almost zero bugs and a minimal manual testing effort. This talk doesn’t deal with the details of Bluetooth on Android. Instead, we will discuss the strategies and methods we used and how it can be applied to any refactoring of dangerous code. Finally, I’m going to show you that you can even write tests for Bluetooth code - there is no excuse not to write tests.

Transcript

1. Refactoring Risky Code Philipp Hofmann
2. Risky Code • Has a low test coverage • Has a high test effort • Has many side effects National Geographic
3. Legacy Code
4. Implementing Bluetooth features on Android can be risky.
5. Bluetooth features on Android are difficult for test automation.
6. You need to test with lots of different devices.
7. We had a stable Bluetooth stack.
8. Bluetooth Stack • It had almost no tests • It was not testable • Could break a lot • Could make users angry National Geographic
9. Make it extendable and changeable.
13. Things You Should Never Do Part 1
14. Refactoring != Rewriting
15. Refactoring
16. Code refactoring is the process of restructuring existing code without changing its external behaviour. Wikipedia
17. Code refactoring is the process of restructuring existing code without changing its external behaviour. Refactoring is intended to improve non functional attributes of the software. Wikipedia
18. Code refactoring is the process of restructuring existing code without changing its external behaviour. Refactoring is intended to improve non functional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve maintainability and create a more expressive internal architecture or object model to improve extensibility. Wikipedia
19. Code refactoring is the process of restructuring existing code without changing its external behaviour. Refactoring is intended to improve non functional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve maintainability and create a more expressive internal architecture or object model to improve extensibility. Wikipedia
20. restructuring existing code without changing its external behaviour improve non functional attributes improved code readability and reduced complexity improve maintainability and extensibility Wikipedia
21. Why should we refactor?
22. “Any fool can write code that a computer can understand. Martin Fowler Good programmers write code that humans can understand.”
23. 70 to 90% of our time we spend on understanding code. Robert C. Martin, Carlola Lilienthal, and The Mythical Man Month
24. fun c( e: List<String> ): HashMap<String, Int> { var i = 0 val r = hashMapOf<String, Int>() while (i < e.size) { val ee = r[e[i]] if (ee != null) { r[e[i]] = ee + 1 } else { r[e[i]] = 1 } i++ } return r }
25. fun countOccurences( elements: List<String> ): HashMap<String, Int> { var i = 0 val results = hashMapOf<String, Int>() while (i < elements.size) { val element = results[elements[i]] if (element != null) { results[elements[i]] = element + 1 } else { results[elements[i]] = 1 } i++ } return results }
26. fun countOccurences( elements: List<String> ): List<Pair<String, Int>> = elements.groupBy { it }.map { Pair(it.key, it.value.count()) }
27. https://cropper.watch.aetnd.com/cdn.watch.aetnd.com/sites/2/2018/06/Einstein-78970955.jpg
29. This time we do it right.
30. Even the best get it wrong sometimes.
31. Loose trust.
32. You have do to the dishes.
33. How to refactor code without breaking it?
34. Tests.
35. Loads of Tests.
36. It’s safer to refactor when your code is covered by tests.
37. Code without tests is a bad situation.
38. It doesn’t matter how pretty or abstracted it is.
39. Code without tests can also be a good situation.
40. If you have testable code add tests then refactor.
41. Bluetooth Stack • It had almost no tests • It was not testable • Could break a lot • Could make users angry National Geographic
42. Refactoring Risky Code ?
43. There are no tests. 
 Neither is the code testable.
45. ?
46. make it testable risky code add tests refactor
47. make it testable risky code
48. Deep coding
49. Get rid of not testable dependencies.
50. Adding abstractions.
51. class Greeting( private val context: Context, private val connectivityManager: ConnectivityManager ) { val greeting = if (isConnected) { context.getString(R.string.greeting_online) } else { context.getString(R.string.greeting_offline) } private val isConnected: Boolean get() { val activeNetworkInfo = connectivityManager.activeNetworkInfo return activeNetworkInfo != null && activeNetworkInfo.isConnected } }
52. class Greeting( private val context: Context, private val connectivityManager: ConnectivityManager ) { val greeting = if (isConnected) { context.getString(R.string.greeting_online) } else { context.getString(R.string.greeting_offline) } private val isConnected: Boolean get() { val activeNetworkInfo = connectivityManager.activeNetworkInfo return activeNetworkInfo != null && activeNetworkInfo.isConnected } }
53. class Connectivity( private val connectivityManager: ConnectivityManager ) { val isConnected: Boolean get() { val activeNetworkInfo = connectivityManager.activeNetworkInfo return activeNetworkInfo != null && activeNetworkInfo.isConnected } }
54. class Greeting( private val context: Context, private val connectivity: Connectivity ) { val greeting = if (connectivity.isConnected) { context.getString(R.string.greeting_online) } else { context.getString(R.string.greeting_offline) } }
55. class Greeting( private val context: Context, private val connectivity: Connectivity ) { val greeting = if (connectivity.isConnected) { context.getString(R.string.greeting_online) } else { context.getString(R.string.greeting_offline) } }
56. interface GreetingResources { val greetingOnline: String val greetingOffline: String } class AndroidGreetingResources( private val context: Context ) : GreetingResources { override val greetingOnline: String get() = context.getString(R.string.greeting_online) override val greetingOffline: String get() = context.getString(R.string.greeting_offline) }
57. class Greeting( private val resources: GreetingResources, private val connectivity: Connectivity ) { val greeting = if (connectivity.isConnected) { resources.greetingOnline } else { resources.greetingOffline } }
59. interface ClassicScanner { suspend fun scan(): ReceiveChannel<ClassicScanResult> }
60. class AndroidClassicScanner( private val bluetoothProvider: BluetoothProvider, private val context: Context ) : ClassicScanner { override suspend fun scan(): ReceiveChannel<ClassicScanResult> { // All the other stuff } }
61. is stupid. contains logic.
62. Refactoring Survival Guide Those tips can save your career.
63. 1. Refactoring. What else?
64. When refactoring we don’t change the behaviour of the code.
65. We improve the structure.
66. I know what I am doing.
67. No you don’t.
68. 2. Keep your code green.
70. Miller Jackson
71. Justin Miller Chris Jackson
72. Justin Miller Frank Chris Jackson Tina
73. Justin Miller Frank Chris Jackson Tina
74. Justin Miller Frank Chris Jackson Tina
75. Justin Miller Frank Chris Jackson Tina
76. Justin Miller Frank Chris Jackson Tina
77. Justin Miller Frank Chris Jackson Tina
78. Justin Miller Frank Chris Jackson Tina
79. Justin Miller Frank Chris Jackson Tina
80. Justin Miller Frank Chris Jackson Tina
81. 3. Small Tasks
83. Don’t go crazy.
84. Clearly define a goal.
85. Small Tasks • Merge often. • Avoid huge PRs.
86. 4. Commit a lot
87. When you reach a clear state, make a commit.
88. git reset HEAD --hard
89. Tiny steps
91. 5. Use the tools.
92. Don’t do stuff manually.
93. Android Studio is your friend.
94. Find & Replace can help.
95. 6. Keep your tests green.
98. Don’t continue with failing tests.
99. 7. Tests. Tests. Tests.
102. enum class Level { Junior, Senior, Ancient } fun bonus(workingMonths: Int, salary: Float, level: Level) = if (workingMonths >= 12) { val bonusMultiplier = when (level) { Level.Junior -> 1.1f Level.Senior -> 1.15f Level.Ancient -> 1.30f } round(salary * bonusMultiplier) } else { salary }
103. @Test fun `junior worked 11 months`() = assertEquals(40000f, bonus(11, 40000f, Level.Junior), 0.01f) @Test fun `junior worked 12 months`() = assertEquals(44000f, bonus(12, 40000f, Level.Junior), 0.01f) @Test fun `senior worked 12 months`() = assertEquals(57500f, bonus(12, 50000f, Level.Senior), 0.01f) @Test fun `ancient worked 12 months`() = assertEquals(65000f, bonus(12, 50000f, Level.Ancient), 0.01f)
104. @Test fun `junior worked 23 months`() = assertEquals(40000f, bonus(23, 40000f, Level.Junior), 0.01f) @Test fun `junior worked 24 months`() = assertEquals(44000f, bonus(24, 40000f, Level.Junior), 0.01f) @Test fun `senior worked 12 months`() = assertEquals(57500f, bonus(12, 50000f, Level.Senior), 0.01f) @Test fun `ancient worked 12 months`() = assertEquals(65000f, bonus(12, 50000f, Level.Ancient), 0.01f)
106. fun bonus(workingMonths: Int, salary: Float, level: Level) = if (workingMonths >= 12) { val bonusMultiplier = when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> 1.15f Level.Ancient -> 1.30f } round(salary * bonusMultiplier) } else { salary }
108. @Test fun `junior worked 23 months`() = assertEquals(40000f, bonus(23, 40000f, Level.Junior), 0.01f) @Test fun `junior worked 24 months`() = assertEquals(44000f, bonus(24, 40000f, Level.Junior), 0.01f) @Test fun `senior worked 11 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Senior), 0.01f) @Test fun `senior worked 12 months`() = assertEquals(57500f, bonus(12, 50000f, Level.Senior), 0.01f) @Test fun `ancient worked 11 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Ancient), 0.01f) @Test fun `ancient worked 12 months`() = assertEquals(65000f, bonus(12, 50000f, Level.Ancient), 0.01f)
110. @Test fun `junior worked 23 months`() = assertEquals(40000f, bonus(23, 40000f, Level.Junior), 0.01f) @Test fun `junior worked 24 months`() = assertEquals(44000f, bonus(24, 40000f, Level.Junior), 0.01f) @Test fun `senior worked 15 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Senior), 0.01f) @Test fun `senior worked 16 months`() = assertEquals(57500f, bonus(16, 50000f, Level.Senior), 0.01f) @Test fun `ancient worked 11 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Ancient), 0.01f) @Test fun `ancient worked 12 months`() = assertEquals(65000f, bonus(12, 50000f, Level.Ancient), 0.01f)
112. fun bonus(workingMonths: Int, salary: Float, level: Level) = if (workingMonths >= 12) { val bonusMultiplier = when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> 1.15f Level.Ancient -> 1.30f } round(salary * bonusMultiplier) } else { salary }
113. fun bonus(workingMonths: Int, salary: Float, level: Level) = salary * when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> if (workingMonths > 12) 1.15f else 1f Level.Ancient -> if (workingMonths >= 12) 1.3f else 1f }
114. fun bonus(workingMonths: Int, salary: Float, level: Level) = salary * when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> if (workingMonths > 16) 1.15f else 1f Level.Ancient -> if (workingMonths >= 12) 1.3f else 1f }
116. @Test fun `junior worked 23 months`() = assertEquals(40000f, bonus(23, 40000f, Level.Junior), 0.01f) @Test fun `junior worked 24 months`() = assertEquals(44000f, bonus(24, 40000f, Level.Junior), 0.01f) @Test fun `senior worked 15 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Senior), 0.01f) @Test fun `senior worked 16 months`() = assertEquals(57500f, bonus(16, 50000f, Level.Senior), 0.01f) @Test fun `ancient worked 11 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Ancient), 0.01f) @Test fun `ancient worked 12 months`() = assertEquals(65000f, bonus(12, 50000f, Level.Ancient), 0.01f)
117. @Test fun `senior worked 15 months`() = assertEquals(50000f, bonus(11, 50000f, Level.Senior), 0.01f)
118. @Test fun `senior worked 15 months`() = assertEquals(50000f, bonus(15, 50000f, Level.Senior), 0.01f)
120. fun bonus(workingMonths: Int, salary: Float, level: Level) = salary * when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> if (workingMonths > 16) 1.15f else 1f Level.Ancient -> if (workingMonths >= 12) 1.3f else 1f }
121. fun bonus(workingMonths: Int, salary: Float, level: Level) = salary * when (level) { Level.Junior -> if (workingMonths >= 24) 1.1f else 1f Level.Senior -> if (workingMonths >= 16) 1.15f else 1f Level.Ancient -> if (workingMonths >= 12) 1.3f else 1f }
123. 8. Minimise The Damage
124. Staged rollout.
125. Use alpha testing.
126. Put your refactoring behind a feature switch.
127. You can roll back within a minute.
128. We duplicated our code and made small changes to it.
129. Secret status view for QA
130. 9. Ship.
131. It only counts when it’s shipped.
132. Conclusion
133. Refactoring is improving the structure of the code without changing its external behaviour.
134. Refactoring Survival Guide 1. Refactoring. What else? 2. Keep your code green. 3. Keep your tests green. 4. Tests. Tests. Tests. 5. Small tasks. 6. Commit a lot. 7. Use the tools. 8. Minimise the damage. 9. Ship.
135. make it testable risky code
136. Deep coding.
137. make it testable risky code
138. make it testable risky code add tests refactor
139. make it testable risky code add tests refactor @muffls Philipp Hofmann