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
   

   View 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 Slide

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

   View Slide

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

   View Slide

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

   View 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 Slide

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

   View Slide

8. What is wrong with my code?
   

   View Slide

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

   View Slide

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

    View Slide

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

    View Slide

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

    View 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 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 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 Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View 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 Slide

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

    View Slide

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

    View Slide

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

    View Slide

24. Take Away
    Always pick string interpolation
    over string concatenation
    

    View 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 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 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 Slide

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

    View 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 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 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 Slide

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

    View 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 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 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 Slide

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

    View Slide

37. Should I use
    non-final case classes?
    

    View 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 Slide

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

    View Slide

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

    View Slide

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

    View Slide

42. Take Away
    Fully name Boolean parameters
    

    View 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 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 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 Slide

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

    View 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 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 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 Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    View Slide

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

    extra layer in your type?
    

    View 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 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 Slide

57. Thank You!
    •Twitter: @DanielaSfregola
    
    •Book: Get Programming With Scala
    
    •Blog: danielasfregola.com
    

    View Slide