Scala Days 2019 - 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 both readable and performant.

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 Days 2019
2. Hellooooo • Software Engineer 
 working with Scala • Ex Java Developer • Author of 
 "Get Programming with Scala" by Manning
 
 40% off all Manning products with code ctwsdays19
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 = "Happy 10th Birthday" a + " Scala Days!"
22. Before val a: String = "Happy 10th Birthday" a + " Scala Days!"
23. After val a: String = "Happy 10th Birthday" s"$a Scala Days!"
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. Bonus Round 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") } Thanks Dr M. Odersky!
30. Bonus Round 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") } Thanks Dr M. Odersky!
31. Bonus Round def extractParameter(args: Array[String]): String = args match { case Array(element) => element case Array() => throw new IllegalArgumentException("No param found") case _ => throw new IllegalArgumentException("Too many params found") } Thanks Dr M. Odersky!
32. Bonus Take Away Code Reviews are the best way to improve your code!
33. 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
34. 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
35. 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 -> Nicolas Rinaudo, tomorrow at 15:45!
36. After final case class A(x: Int)
37. Should I use non-final case classes?
38. Take Away Always make your case classes final* ! If you need to extend them, 
 consider converting your case classes to regular classes. *Unless you have a really good reason not to
39. Before def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(true, false)
40. Before def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(true, false)
41. After def doSomething(enableA: Boolean, enableB: Boolean): Unit = ??? doSomething(enableA = true, enableB = false)
42. Take Away Fully name Boolean parameters
43. 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
44. 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
45. After def doSomething(): Future[Int] = for { resB <- b() } yield a() + resB + c() private def a(): Int = 42 private def b(): Future[Int] = { val dataFromDb: Future[Int] = ??? dataFromDb } private def c(): Int = 24
46. Take Away Never ever block Future ! If you cannot avoid it, try to do it as late as possible
47. 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) }
48. 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) }
49. 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) }
50. Take Away Recursive function? Try to make it tail recursive and use the @tailrec annotation
51. Before val myMap: Option[Map[String, String]] = ??? val myList: Option[List[String]] = ??? val myOptOpt: Option[Option[String]] = ??? val myBoolean: Option[Boolean] = ???
52. Before val myMap: Option[Map[String, String]] = ??? val myList: Option[List[String]] = ??? val myOptOpt: Option[Option[String]] = ??? val myBoolean: Option[Boolean] = ???
53. After val myMap: Map[String, String] = ??? val myList: List[String] = ??? val myOptOpt: Option[String] = ??? val myBoolean: Boolean = ???
54. Take Away Try to simplify your types . Do you really need that 
 extra layer in your type?
55. 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)
56. 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
57. Thank You! •Twitter: @DanielaSfregola •Book: Get Programming With Scala •Blog: danielasfregola.com