Refactor all the things!

Learning the syntax is just the first step towards mastering a new language. Idiomatic expressions and good practices must also be adopted to produce code that is readable and performant. Without guidance on its specific style, you can quickly develop habits that could cause your application to be difficult to reason about and maintain.

In this talk, we'll analyse snippets of code and highlight common Scala anti-patterns that make them difficult to understand. We'll also discuss how to refactor them to improve their readability. At the end of this session, you are going to be able to recognise code smells and refactor your code to make it easier to reason about and maintain, as well as avoid common pitfalls and possible bugs.

Transcript

1. Refactor All The Things! @DanielaSfregola Scala Exchange 2018
2. Hellooooo • Software Engineer 
 working with Scala • Ex Java Developer • London Scala User Group • Author of 
 "Get Programming with Scala" by Manning
3. WTFs/min from www.osnews.com/story/19266/WTFs_m
4. from https://devrant.com/rants/811491/git-blame-strikes-again
5. from https://cheezburger.com/8311162880
6. "Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live." by John F. Woods, 1991
7. by Rory Graves, 2018 "The hardest problem in computer science is avoiding accidental complexity - complexity unrelated to the problem at hand"
8. What is wrong with my code?
9. Before object Database { private def connectToDatabase(config: Config): Connection = ??? def connection: Connection = { val config: Config = ??? connectToDatabase(config) } }
10. Before object Database { private def connectToDatabase(config: Config): Connection = ??? def connection: Connection = { val config: Config = ??? connectToDatabase(config) } }
11. After object Database { private def connectToDatabase(config: Config): Connection = ??? val connection: Connection = { val config: Config = ??? connectToDatabase(config) } }
12. Take Away Function with no parameters? Consider replacing def with val to avoid excessive memory allocation
13. Before class MyApi { def myEntryPoint(data: Data): OtherData = { // some stuff here anotherFunction(data) anotherHelpFunction(data) } def anotherFunction(data: Data) = ??? def anotherHelpFunction(data: Data) = ??? }
14. Before class MyApi { def myEntryPoint(data: Data): OtherData = { // some stuff here anotherFunction(data) anotherHelpFunction(data) } def anotherFunction(data: Data) = ??? def anotherHelpFunction(data: Data) = ??? }
15. After class MyApi { def myEntryPoint(data: Data): OtherData = { // some stuff here anotherFunction(data) anotherHelpFunction(data) } private def anotherFunction(data: Data) = ??? private def anotherHelpFunction(data: Data) = ??? }
16. Take Away Use the most restrictive access modifier that makes your code work! Use private and protected whenever possible
17. Before def myFunction(): Unit = { val data: Int = 42 // do something here data }
18. Before def myFunction(): Unit = { val data: Int = 42 // do something here data }
19. After def myFunction(): Int = { val data: Int = 42 // do something here data }
20. Take Away Enable compiler flags to detect 
 suspicious patterns in your code. See Recommended Scalac Flags by Rob Norris scalacOptions += "-Xfatal-warnings" // add to your build.sbt
21. Before val a: String = "Hello" a + " ScalaExchange!"
22. Before val a: String = "Hello" a + " ScalaExchange!"
23. After val a: String = "Hello" s"$a ScalaExchange!"
24. Take Away Always pick string interpolation over string concatenation
25. Before def extractParameter(args: Array[String]): String = if (args.length == 0) { throw new IllegalArgumentException("No param found") } else if (args.length >= 1) { throw new IllegalArgumentException("Too many params found") } else args.head
26. Before def extractParameter(args: Array[String]): String = if (args.length == 0) { throw new IllegalArgumentException("No param found") } else if (args.length >= 1) { throw new IllegalArgumentException("Too many params found") } else args.head
27. After def extractParameter(args: Array[String]): String = args match { case Array() => throw new IllegalArgumentException("No param found") case Array(element) => element case _ => throw new IllegalArgumentException("Too many params found") }
28. Take Away Avoid unsafe functions, such as Collection.head (and Option.get) ! Consider using pattern matching and high order functions instead.
29. Before case class A(x: Int) class B(x: Int, y: Int) extends A(x) case class A(x: Int) class B(x: Int, y: Int) extends A(x) Before
30. Before case class A(x: Int) class B(x: Int, y: Int) extends A(x) case class A(x: Int) class B(x: Int, y: Int) extends A(x) Before
31. Before case class A(x: Int) class B(x: Int, y: Int) extends A(x) scala> new B(2, 1) == A(2) res1: Boolean = true WHAT !?! full explanation on stackoverflow
32. After final case class A(x: Int)
33. Take Away Always make your case classes final ! If you need to extend them, 
 consider converting your case classes to regular classes.
34. Before def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(true, false)
35. Before def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(true, false)
36. After def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(enableA = true, enableB = false)
37. Take Away Fully name Boolean parameters
38. Before def doSomething(): Int = a() + b() + c() private def a(): Int = 42 private def b(): Int = { val dataFromDb: Future[Int] = ??? Await.result(dataFromDb, Duration.Inf) } private def c(): Int = 24
39. Before def doSomething(): Int = a() + b() + c() private def a(): Int = 42 private def b(): Int = { val dataFromDb: Future[Int] = ??? Await.result(dataFromDb, Duration.Inf) } private def c(): Int = 24
40. After def doSomething(): Future[Int] = for { resA <- Future.successful(a()) resB <- b() resC <- Future.successful(c()) } yield resA + resB + resC private def a(): Int = 42 private def b(): Future[Int] = { val dataFromDb: Future[Int] = ??? dataFromDb } private def c(): Int = 24
41. Take Away Never ever block Future ! If you cannot avoid it, try to do it as late as possible
42. Before def factorial(number: Int): Int = { def loop(n: Int, acc: Int): Int = n match { case 0 => 0 case 1 => acc case x => loop(x - 1, x * acc) } loop(number, acc = 1) }
43. Before def factorial(number: Int): Int = { def loop(n: Int, acc: Int): Int = n match { case 0 => 0 case 1 => acc case x => loop(x - 1, x * acc) } loop(number, acc = 1) }
44. After def factorial(number: Int): Int = { @tailrec def loop(n: Int, acc: Int): Int = n match { case 0 => 0 case 1 => acc case x => loop(x - 1, x * acc) } loop(number, acc = 1) }
45. Take Away Recursive function? Try to make it tail recursive and use the @tailrec annotation
46. Before val myMap: Option[Map[String, String]] = ??? val myList: Option[List[String]] = ??? val myOptOpt: Option[Option[String]] = ??? val myBoolean: Option[Boolean] = ???
47. Before val myMap: Option[Map[String, String]] = ??? val myList: Option[List[String]] = ??? val myOptOpt: Option[Option[String]] = ??? val myBoolean: Option[Boolean] = ???
48. After val myMap: Map[String, String] = ??? val myList: List[String] = ??? val myOptOpt: Option[String] = ??? val myBoolean: Boolean = ???
49. Take Away Try to simplify your types . Do you really need that 
 extra layer in your type?
50. Take Aways •Consider replacing def with val 
 to avoid excessive memory allocation •Use private and protected whenever possible •Enable compiler flags to detect 
 suspicious patterns in your code. •Always pick string interpolation 
 over string concatenation •Avoid unsafe functions, 
 such as Collection.head (and Option.get)
51. Take Aways (2) •Always make your case classes final •Fully name Boolean parameters •Never ever block Future •Use the @tailrec annotation •Try to simplify your types
52. Summary •Refactor your code 
 to improve your coding skills •Refactor for the sanity of the people maintaining your code •Refactor to avoid 
 accidental complexity
53. Thank You! •Twitter: @DanielaSfregola •Blog: danielasfregola.com •London Scala User Group (LSUG)