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

Scala Days 2019 - Refactor all the things!

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.

Daniela Sfregola

June 12, 2019
Tweet

More Decks by Daniela Sfregola

Other Decks in Technology

Transcript

  1. Refactor
    All The Things!
    @DanielaSfregola

    Scala Days 2019

    View full-size slide

  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

    View full-size slide

  3. WTFs/min
    from www.osnews.com/story/19266/WTFs_m

    View full-size slide

  4. from https://devrant.com/rants/811491/git-blame-strikes-again

    View full-size slide

  5. from https://cheezburger.com/8311162880

    View full-size slide

  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

    View full-size slide

  7. by Rory Graves, 2018
    "The hardest problem in computer science is
    avoiding accidental complexity - complexity
    unrelated to the problem at hand"

    View full-size slide

  8. What is wrong with my code?

    View full-size slide

  9. Before
    object Database {
    private def connectToDatabase(config: Config): Connection = ???
    def connection: Connection = {
    val config: Config = ???
    connectToDatabase(config)
    }
    }

    View full-size slide

  10. Before
    object Database {
    private def connectToDatabase(config: Config): Connection = ???
    def connection: Connection = {
    val config: Config = ???
    connectToDatabase(config)
    }
    }

    View full-size slide

  11. After
    object Database {
    private def connectToDatabase(config: Config): Connection = ???
    val connection: Connection = {
    val config: Config = ???
    connectToDatabase(config)
    }
    }

    View full-size slide

  12. Take Away
    Function with no parameters?
    Consider replacing def with val to avoid
    excessive memory allocation

    View full-size slide

  13. Before
    class MyApi {
    def myEntryPoint(data: Data): OtherData = {
    // some stuff here
    anotherFunction(data)
    anotherHelpFunction(data)
    }
    def anotherFunction(data: Data) = ???
    def anotherHelpFunction(data: Data) = ???
    }

    View full-size slide

  14. Before
    class MyApi {
    def myEntryPoint(data: Data): OtherData = {
    // some stuff here
    anotherFunction(data)
    anotherHelpFunction(data)
    }
    def anotherFunction(data: Data) = ???
    def anotherHelpFunction(data: Data) = ???
    }

    View full-size slide

  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) = ???
    }

    View full-size slide

  16. Take Away
    Use the most restrictive access modifier that
    makes your code work!
    Use private and protected whenever possible

    View full-size slide

  17. Before
    def myFunction(): Unit = {
    val data: Int = 42
    // do something here
    data
    }

    View full-size slide

  18. Before
    def myFunction(): Unit = {
    val data: Int = 42
    // do something here
    data
    }

    View full-size slide

  19. After
    def myFunction(): Int = {
    val data: Int = 42
    // do something here
    data
    }

    View full-size slide

  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

    View full-size slide

  21. Before
    val a: String = "Happy 10th Birthday"
    a + " Scala Days!"

    View full-size slide

  22. Before
    val a: String = "Happy 10th Birthday"
    a + " Scala Days!"

    View full-size slide

  23. After
    val a: String = "Happy 10th Birthday"
    s"$a Scala Days!"

    View full-size slide

  24. Take Away
    Always pick string interpolation
    over string concatenation

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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")
    }

    View full-size slide

  28. Take Away
    Avoid unsafe functions,
    such as Collection.head (and Option.get) !
    Consider using pattern matching
    and high order functions instead.

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  32. Bonus Take Away
    Code Reviews
    are the best way
    to improve your code!

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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!

    View full-size slide

  36. After
    final case class A(x: Int)

    View full-size slide

  37. Should I use
    non-final case classes?

    View full-size slide

  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

    View full-size slide

  39. Before
    def doSomething(enableA: Boolean, enableB: Boolean): Unit = ???
    doSomething(true, false)

    View full-size slide

  40. Before
    def doSomething(enableA: Boolean, enableB: Boolean): Unit = ???
    doSomething(true, false)

    View full-size slide

  41. After
    def doSomething(enableA: Boolean, enableB: Boolean): Unit = ???
    doSomething(enableA = true, enableB = false)

    View full-size slide

  42. Take Away
    Fully name Boolean parameters

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  46. Take Away
    Never ever block Future !
    If you cannot avoid it,
    try to do it as late as possible

    View full-size slide

  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)
    }

    View full-size slide

  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)
    }

    View full-size slide

  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)
    }

    View full-size slide

  50. Take Away
    Recursive function?
    Try to make it tail recursive
    and use the @tailrec annotation

    View full-size slide

  51. Before
    val myMap: Option[Map[String, String]] = ???
    val myList: Option[List[String]] = ???
    val myOptOpt: Option[Option[String]] = ???
    val myBoolean: Option[Boolean] = ???

    View full-size slide

  52. Before
    val myMap: Option[Map[String, String]] = ???
    val myList: Option[List[String]] = ???
    val myOptOpt: Option[Option[String]] = ???
    val myBoolean: Option[Boolean] = ???

    View full-size slide

  53. After
    val myMap: Map[String, String] = ???
    val myList: List[String] = ???
    val myOptOpt: Option[String] = ???
    val myBoolean: Boolean = ???

    View full-size slide

  54. Take Away
    Try to simplify your types .
    Do you really need that 

    extra layer in your type?

    View full-size slide

  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)

    View full-size slide

  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

    View full-size slide

  57. Thank You!
    •Twitter: @DanielaSfregola

    •Book: Get Programming With Scala

    •Blog: danielasfregola.com

    View full-size slide