Refactor all the things!

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.

E99b07644586e9e1723757bf8e34ea68?s=128

Daniela Sfregola

December 14, 2018
Tweet

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)