Refactor

Ugly Scala Code

Code Weaver

By

Karl Roberts

@MrK4rl

Demo code

• can be found at the link below
• https://github.com/karlroberts/scalasydUglyCode

Confession

• First I need to admit to a dirty little secret I’ve been keeping…

The Ugly Truth

The Ugly Truth

• It can be easier to hack away in an imperative style
•  
•  

The Ugly Truth

• It can be easier to hack away in an imperative style
• Especially when hacking on a new Java API
•  

The Ugly Truth

• It can be easier to hack away in an imperative style
• Especially when hacking on a new Java API
• Or noodling around to get an understanding

The Excuses?

The Excuses?

• Thinking in Java
•    
  •  
  •  
•  

The Excuses?

• Thinking in Java
• looping in loops
  • helps walk or build a data structure
  • while I’m developing it
•  

The Excuses?

• Thinking in Java
• looping in loops
  • helps walk or build a data structure
  • while I’m developing it
• Types get in the way less

The Problem?

• Sneaky Bugs can exist
  •  
• Hard to test pieces in isolation
• Hard to understand later or refactor?
  •  
•  

The Problem?

• Sneaky Bugs can exist
  • especially in the edge cases
• Hard to test pieces in isolation
• Hard to understand later or refactor?
  •  
•  

The Problem?

• Sneaky Bugs can exist
  • especially in the edge cases
• Hard to test pieces in isolation
• Hard to understand later or refactor?
  • you may forget to take account of some out of view mutation
•  

The Problem?

• Sneaky Bugs can exist
  • especially in the edge cases
• Hard to test pieces in isolation
• Hard to understand later or refactor?
  • you may forget to take account of some out of view mutation
• Gives me the creeps.

The Solution?

• Once Running Refactor
  • while you still understand the problem
• Remove mutation
• Remove loops
• Learn the API you are working in
  • it may well have realy helpful methods/functions for you to use.
• Discover nice general patterns

My Example?

source code location

PriceTable

• I need to build up a PriceTable for Currencies.
  •  
•  
  •  
  •  

PriceTable

• I need to build up a PriceTable for Currencies.
  • A Map of Maps
•  
  •  
  •  

PriceTable

• I need to build up a PriceTable for Currencies.
  • A Map of Maps
• So I can
  •  
  •  

PriceTable

• I need to build up a PriceTable for Currencies.
  • A Map of Maps
• So I can
  • look up a currency and
  •  

Build a PriceTable

• I need to build up a PriceTable for Currencies.
  • A Map of Maps
• So I can
  • look up a currency and
  • from there look up a price a date I care about

PriceTable

// a type alias
type PriceTable = Map[Currency, PriceTableForCurrency]

PriceTableForCurrency

source code location

case class PriceTableForCurrency(currency: Currency, prices: Map[LocalDateTime,Double]) {
  /**
    * Will overwrite if we alredy have a price at this time
    * @param historicPrice
    * @return
    */
  def +=(historicPrice: HistoricPrice) = {
    val hp = (historicPrice.localDateTime, historicPrice.quotePrice)
    this.copy(prices = prices + hp)
  }
  def ++=(other:PriceTableForCurrency) = if(other.currency == currency) this.copy(prices = prices ++ other.prices) else this

  def asHistoricPrices: List[HistoricPrice] = {
    import scala.collection.mutable.ListBuffer
    val lb = ListBuffer[HistoricPrice]()
    prices.foreach(tup => {
      lb += HistoricPrice(tup._1,currency,tup._2)
    })
    lb.toList
  }
}

My Example - quick re-read for context

source code location

Lets create a better Map of Maps

Api docs for Map

• We can contruct a Map like this:

  val myMap = Map(“a” -> “little A”, “b” -> “little B”)

•  

•  
  •  

Lets create a better Map of Maps

Api docs for Map

• We can contruct a Map like this:

  val myMap = Map(“a” -> “little A”, “b” -> “little B”)

• The -> is a function that create a Tuple

•  
  •  

Lets create a better Map of Maps

Api docs for Map

• We can contruct a Map like this:

  val myMap = Map(“a” -> “little A”, “b” -> “little B”)

• The -> is a function that create a Tuple

• and so this is like building a Map from a list of Tuples
  • ie (Key, Value) pairs

Lets create a better Map of Maps

Api docs for Map

• We can contruct a Map like this:

  val myMap = Map(“a” -> “little A”, “b” -> “little B”)

• The -> is a function that create a Tuple

• and so this is like building a Map from a list of Tuples
  • ie (Key, Value) pairs

Lets create a better Map of Maps

Api docs for List

• If only there was a way to turn a list of tuples into a Map

•  

Lets create a better Map of Maps

Api docs for List

• If only there was a way to turn a list of tuples into a Map

• 

Lets create a better Map of Maps

Api docs for List

• So we can build a map from a list but what about a Map of Maps?

•  

•  

Lets create a better Map of Maps

Api docs for List

• So we can build a Map from a list but what about a Map of Maps?

• If only there was a way to group list items by some attribute

•  

Lets create a better Map of Maps

Api docs for List

• So we can build a Map from a list but what about a Map of Maps?

• If only there was a way to group list items by some attribute

• 

Lets create a better Map of Maps

Api docs for List

• So this is cool we an turn a List of Tuple2 into a Map

• We can also group a List into a Map with a List as its values

• So if the value List could be transformed into a Tuple

  • we have th building blocks for a Map of Maps

Lets do it - fix Map of Maps trial 1

• git checkout start_1

Lets do it - fix Map of Maps trail 2

• git checkout start_2

Lets do it - fixed Map of Maps

• git checkout start_3

Lets fix that ugly foreach

• Instead of trying to do 3 things
  •  
  •  
  •  
    •  
    •  
    •  

Lets fix that ugly foreach

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  •  
  •  
    •  
    •  
    •  

Lets fix that ugly foreach:

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  • fetch the new historic price
  •  
    •  
    •  
    •  

Lets fix that ugly foreach:

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  • fetch the new historic price
  • add it to Map of Maps
    •  
    •  
    •  

Lets fix that ugly foreach:

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  • fetch the new historic price
  • add it to Map of Maps
    • looking out to see if we have the key or not first
    •  
    •  

Lets fix that ugly foreach:

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  • fetch the new historic price
  • add it to Map of Maps
    • looking out to see if we have the key or not first
    • and if not adding else just adding
    •  

Lets fix that ugly foreach:

• Instead of trying to do 3 things
  • walk trades looking for a historic price miss
  • fetch the new historic price
  • add it to Map of Maps
    • looking out to see if we have the key or not first
    • and if not adding else just adding
    • again looking to see if we have the nested key etc..

Lets fix that ugly foreach - why not this?

Lets fix that ugly foreach - why not this?

• Filter the list of trades
  •  
•  
•  

Lets fix that ugly foreach - why not this?

• Filter the list of trades
  • to create a List of trades that dont have a price
•  
•  

Lets fix that ugly foreach - why not this?

• Filter the list of trades
  • to create a List of trades that dont have a price
• Then just get them
• I can worry about how to stuff the new prices in the PriceTable later

Lets do it

• git checkout start_4

Add to imutable PriceTable

• use a fold
• have the starting accumulator be the current PriceTable

so …?

• We’ve tidied most of it up
  • by mapping and filtering and folding we dont need var placeholders anymore

•  

•  

•  
  •  

so …?

• We’ve tidied most of it up
  • by mapping and filtering and folding we dont need var placeholders anymore

• Can I get onto the fun stuff?

•  

•  
  •  

so can I prematurely optimize yet?

• We’ve tidied most of it up
  • by mapping and filtering and folding we dont need var placeholders anymore

• Can I get onto the fun stuff?

• so can i tune it up a bit?

•  
  •  

so can I prematurely optimize yet?

• We’ve tidied most of it up
  • by mapping and filtering and folding we dont need var placeholders anymore

• Can I get onto the fun stuff?

• Can i tune it up a bit?

• Yes by fetching in parallel.
  • which is now trivial as I have simple collections to work with.

so can I prematurely optimize yet?

• I will use scalaz Task
• to wrap the get historic prices function
• and return a List[Task[PriceTableForCurrency]]
• rather than a List[PriceTableForCurrency]
• then gather all the tasks in parallel
• to give me back a List[PriceTableForCurrency]

Lets do it

$ git checkout start_5
$ # clear out the existing prefetched prices
$ cat /dev/null >  $HOME/uglycode/coinspotpos/historicPrices.csv
$ sbt clean run

Ugly?

source code location

Pretty?

source code location

The End

Thanks ….

• me @MrK4rl

• code used in demo https://github.com/karlroberts/scalasydUglyCode

• deck and talk (http://karlcode.owtelse.com/blog/2018/05/09/refactoring-ugly-scala-to-idiomatic-scala/?mode=deck#slide-0)
• blog (http://karlcode.owtelse.com/blog/2018/05/09/refactoring-ugly-scala-to-idiomatic-scala/?mode=doc#slide-0)