3

In several different places in my application, I need to take a Seq[SalesRow] and return a Map[String,SalesRow], where the string is the name of a country.

I need to use this in several places. For example, I take a list of all SalesRows and get a global breakdown of sales by country. But in other places, I want to break down my sales by month and then by country (so Map[Month,Seq[SalesRow]] becomes Map[Month,Map[String,Seq[SalesRow]]]) - in still other places, I want to break down by day and then by country.

My question is: where do I put the (small) amount of logic that takes a Seq[SalesRow] and returns a map of countries to rows? Right now, I'm putting it in a companion object method, SalesRow.byCountry(rows : Seq[SalesReport]. Is that optimal?

A slightly crazier idea occurred to me, which is to create an implicit conversion from Seq[SalesRow] to EnhancedSalesRowSeq, which has a byCountry instance method. This appeals to me, because the operation is applicable to any sequence of SalesRows.

Is this a good idea?

Is adding the logic to the companion object my best choice, or are there better options?

Thanks.

Bill
  • 44,502
  • 24
  • 122
  • 213

3 Answers3

2

If performance isn´t your main concern, you can put the logic at:

class RichTraversable[A](t: Traversable[A]) {
  def toMapBy[B](f: A => B): Map[B,A] = t.map{ e => (f(e),e) }.toMap
}

So if an implicit conversion you can turn every Seq into a Map with a member being the key.

Peter Schmitz
  • 5,824
  • 4
  • 26
  • 48
  • Right, that's what I'm proposing in the second-to-last paragraph. My question is: is this a really weird or unexpected thing to do? – Bill May 01 '11 at 01:20
  • This is more or less equivalent to `groupBy` which is a library defined method. – huynhjl May 01 '11 at 02:06
  • It isn´t unexpected as you can infer from @huynhjl comment, that it is an oversight of me reimplmenting a library method. – Peter Schmitz May 01 '11 at 08:19
  • Ok, my method implies uniqueness by key, so the values are one `B` element and not a `Seq[B]` compared to the library method – Peter Schmitz May 01 '11 at 16:02
2

In case you are not aware the library comes with a groupBy function. Basically given a Seq[SalesRow] it will give you a Map[T, Seq[SalesRow]] based on a function from SalesRow to T.

So if your function is easy you can easily get a map. I do like your idea of the enhanced seq in conjunction with putting the implicit in the SalesRow companion:

case class SalesRow(val month:Int, val country:String, 
  val person:String, val amount:Float)

class EnhancedRow(rows: Seq[SalesRow]) {
  def byCountry: Map[String, Seq[SalesRow]] = 
    rows.groupBy(_.country)
  def byMonth: Map[Int, Seq[SalesRow]] = 
    rows.groupBy(_.month)
  def byCountryByMonth: Map[String, Map[Int, Seq[SalesRow]]] = byCountry.mapValues(r => new EnhancedRow(rows).byMonth)
}

object SalesRow {
  implicit def toEnhanced(rows: Seq[SalesRow]) = new EnhancedRow(rows) 
}

object Test {
  def main(args:Array[String] = null) {
    val seq: Seq[SalesRow] = // ... fill this
    println(seq.byCountry)
    println(seq.byCountryByMonth)
    // same as:
    println(seq.byCountry.mapValues(_.byMonth))
  }
}
huynhjl
  • 41,520
  • 14
  • 105
  • 158
1

I suggest you make encapsulating classes:

case class AllSalesTable(rows: Seq[SalesRow]) {
  def toSalesByCountry: SalesByCountry
}

case class ContrySalesTable(rows: Seq[SalesRow])

case class SalesByCountry(map: Map[String, CountrySalesTable])

Having someplace to put the method is one benefit, but another benefit is that you'll have greater type safety at the cost of a simple ".rows" here and there.

Daniel C. Sobral
  • 295,120
  • 86
  • 501
  • 681
  • I agree with Daniel! My experience is that having classes or type defs even for simple data structures gives clearer code and the compiler often can help. – Peter Schmitz May 01 '11 at 17:55