4
\$\begingroup\$

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)
   }
 }
\$\endgroup\$
  • 1
    \$\begingroup\$ I don't know Scala, I think your SQL looks fine. I would really like to see someone with Scala knowledge answer this. \$\endgroup\$ – Phrancis Jun 2 '14 at 2:56
  • \$\begingroup\$ @Phrancis I haven't heard from anyone. I'm currently working on my own solution, which is a bit outside of the scope of the original question. I'm implementing a ResultSetParser that allows you to specify one hub parser and 0...n spoke parsers. Results are grouped by hub items. Each spoke parser populates a HashSet attached to the hub. (So you can do SQL(...).as( hub[Bill](billParser) seive[Activity](activityParser) seive[Material]("materialParser) and get the equivalent of a Seq[(Bill, Set[Activity],Set[Activity])]. \$\endgroup\$ – FullTimeCoderPartTimeSysAdmin Jun 2 '14 at 17:17
  • \$\begingroup\$ I think you should post your own solution as an answer, once it is ready, I'm sure it would benefit someone in the future! \$\endgroup\$ – Phrancis Jun 2 '14 at 18:03
  • \$\begingroup\$ I'm not sure why you don't issue 3 separate SQL queries, one for each table, all using the Bill primary key. There is absolutely no benefit having the database do the joins, nor in having Anorm tear everything apart. \$\endgroup\$ – Bob Dalgleish Aug 29 '14 at 17:10
  • \$\begingroup\$ @bob dalgleish I simplified my example. An actual praftice, the join foreign keys are not all the bill pk, meaning that multiple queries would have to be sequenced. Also what about the overhead and network latency involved in a query? Wouldn't that make the 3 query approach slower? \$\endgroup\$ – FullTimeCoderPartTimeSysAdmin Aug 29 '14 at 20:56
2
\$\begingroup\$

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

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)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.