HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Neatly Transforming Anorm ResultSet Into Map[Parent , (Set[Child1], Set[Child2]))]

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mapintotransforminganormparentchild2child1neatlyresultsetset

Problem

I have a table Bill containing Bills with 0...n relationships to Material and Activity (line items on the bill).

My SQL query is essentially bill left join material left join activity.

I have the following insanely tangled code to transform the results into a Map[Bill, (HashSet[Activity], HashSet[Material])].

What's wrong with this code? Is this whole approach wrong? I'm new to Scala and the functional world, but it doesn't seem like this could be the right way.

SQL(
          """
            |SELECT
            | b.id, b.dateDue
            | ,a.id as activityId, a.notes, a.startTime, a.endTime
            | ,m.id as materialId, m.cost, m.description, m.storageLocation
            |FROM Bill b
            |LEFT JOIN Activity a ON a.bill_id = b.id
            |LEFT JOIN Material m ON m.bill_id = b.id
            |WHERE b.id = {id}
          """.stripMargin)
        .on("id" -> id)
        .as(betterParser *)
        .groupBy( x=> (x._1, x._2))
        .map( q => q._1 -> 
                q._2.foldLeft( (HashSet[Activity](), HashSet[Material]()) ) 
                ( (acc, item) =>  (acc._1 + item._3, acc._2 + item._4)))


betterParser transforms each row from the query into a (Long, DateTime, Activity, Material) where the (Long, DateTime) portion are overall bill poperties.

val betterParser = {
    billParser ~ activityParser ~ materialParser map {
      case bill ~ activity ~ material =>
       (bill._1, bill._2, activity, material)
   }
 }

Solution

I was hoping someone would review your scala but I don't think it's going to happen. So I thought I would at least say something about sql.

Alias names

Table aliases are handy especially for very long table names, however I feel that your naming is too minimal. Single-letter aliases tell Mr. Maintainer nothing about what the table means, and it can get confusing if you have multiple tables starting with the same letter. b, a, m are not very useful. Instead I would use Bill, Act, Mat or something similar.

Formatting

Your formatting is not bad, I see you've kept columns from the same table on the same line and that's good; but inline SQL can be a bit difficult to read especially when you introduce column aliases. My recommendation is to use a line break after each column you SELECT. I also noticed a bit of inconsistency in capitalization of key words.

This:

SQL(
          """
            |SELECT
            | b.id, b.dateDue
            | ,a.id as activityId, a.notes, a.startTime, a.endTime
            | ,m.id as materialId, m.cost, m.description, m.storageLocation
            |FROM Bill b
            |LEFT JOIN Activity a ON a.bill_id = b.id
            |LEFT JOIN Material m ON m.bill_id = b.id
            |WHERE b.id = {id}
          """.stripMargin)


I would instead write like this:

SQL(
          """
            |SELECT Bill.id
            | ,Bill.dateDue
            | ,Act.id AS activityId
            | ,Act.notes
            | ,Act.startTime
            | ,Act.endTime
            | ,Mat.id AS materialId
            | ,Mat.cost
            | ,Mat.description
            | ,Mat.storageLocation
            |FROM Bill
            |LEFT JOIN Activity AS Act ON Act.bill_id = Bill.id
            |LEFT JOIN Material AS Mat ON Mat.bill_id = Bill.id
            |WHERE Bill.id = {id}
          """.stripMargin)

Code Snippets

SQL(
          """
            |SELECT
            | b.id, b.dateDue
            | ,a.id as activityId, a.notes, a.startTime, a.endTime
            | ,m.id as materialId, m.cost, m.description, m.storageLocation
            |FROM Bill b
            |LEFT JOIN Activity a ON a.bill_id = b.id
            |LEFT JOIN Material m ON m.bill_id = b.id
            |WHERE b.id = {id}
          """.stripMargin)
SQL(
          """
            |SELECT Bill.id
            | ,Bill.dateDue
            | ,Act.id AS activityId
            | ,Act.notes
            | ,Act.startTime
            | ,Act.endTime
            | ,Mat.id AS materialId
            | ,Mat.cost
            | ,Mat.description
            | ,Mat.storageLocation
            |FROM Bill
            |LEFT JOIN Activity AS Act ON Act.bill_id = Bill.id
            |LEFT JOIN Material AS Mat ON Mat.bill_id = Bill.id
            |WHERE Bill.id = {id}
          """.stripMargin)

Context

StackExchange Code Review Q#50961, answer score: 2

Revisions (0)

No revisions yet.