Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Refactoring Risky Code

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.

Philipp Hofmann

September 09, 2019
Tweet

More Decks by Philipp Hofmann

Other Decks in Technology

Transcript

  1. Risky Code • Has a low test coverage • Has

    a high test effort • Has many side effects National Geographic
  2. Bluetooth Stack • It had almost no tests • It

    was not testable • Could break a lot • Could make users angry National Geographic
  3. 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
  4. 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
  5. 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
  6. restructuring existing code without changing its external behaviour improve non

    functional attributes improved code readability and reduced complexity improve maintainability and extensibility Wikipedia
  7. “Any fool can write code that a computer can understand.

    Martin Fowler Good programmers write code that humans can understand.”
  8. 70 to 90% of our time we spend on understanding

    code. Robert C. Martin, Carlola Lilienthal, and The Mythical Man Month
  9. 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 }
  10. 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 }
  11. Bluetooth Stack • It had almost no tests • It

    was not testable • Could break a lot • Could make users angry National Geographic
  12. ?

  13. 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 } }
  14. 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 } }
  15. class Connectivity( private val connectivityManager: ConnectivityManager ) { val isConnected:

    Boolean get() { val activeNetworkInfo = connectivityManager.activeNetworkInfo return activeNetworkInfo != null && activeNetworkInfo.isConnected } }
  16. 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) } }
  17. 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) } }
  18. 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) }
  19. class Greeting( private val resources: GreetingResources, private val connectivity: Connectivity

    ) { val greeting = if (connectivity.isConnected) { resources.greetingOnline } else { resources.greetingOffline } }
  20. class AndroidClassicScanner( private val bluetoothProvider: BluetoothProvider, private val context: Context

    ) : ClassicScanner { override suspend fun scan(): ReceiveChannel<ClassicScanResult> { // All the other stuff } }
  21. 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 }
  22. @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)
  23. @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)
  24. 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 }
  25. @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)
  26. @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)
  27. 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 }
  28. 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 }
  29. 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 }
  30. @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)
  31. 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 }
  32. 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 }
  33. 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.