4

This code is creating a query for use in retrieving a user's profile on a web back end. It creates a query that assembles the necessary information into a DTO (which is just a case class) that is subsequently sent back as JSON.

  def getProfile(userId: Long)={
    val q = for{
    ((((u,p),a),b), ba) <- filterById(userId) join
               People on (_.personId === _.id) joinLeft
               Addresses on (_._2.addressId === _.id) joinLeft
               Businesses on (_._1._2.businessId === _.id) joinLeft
               Addresses on (_._2.flatMap(_.addressId) === _.id)
    }yield(p, a, b, ba)

    db.run(q.result.headOption).map{ _.map{case(p,a,b,ba) =>
      val business = b match {
      case Some(b) => Some(dtos.Business(b.name, b.abn, b.adminFee, ba, b.id))
      case _ => None
    }

    dtos.ProfileInfo(p, a, business)
  }}
}

I've included the result processing (db.run(...)) for context only.

I'm looking for a more readable way to express the query construction.

My experience reading this is "Wait, what?? ... on (_._1._2.flatMap(_.addressId) .... what is that doing?? Why flatmap there and not here: on (_._1._2.businessId. These are actually straight forwards things, but don't read at all straight fowards.

I'm looking for a way of expressing this that doesn't require the amount of deduction needed to read this version. I have to "deduce" what _._1._2 is, and why it needs to be flattened, which I don't have to do with the equivalent SQL.

Notes:

  • This code comes from an existing application (not written by me) which I am extending.
  • Users, People, Addresses, Businesses are (obviously?) the tables.
  • People and Businesses have Addresses.
  • Users have a Person(*), People have a Business
  • filterByUserId(userId) is basically equivalent to Users.filter(_.id === userId)
  • The equivalent SQL is:

    select p.*, a1.*, b.*, a2.* from Users u 
    innerJoin People p on (u.personId == p.id) 
    leftJoin Addresses a1 on (p.addressId == a1.id) 
    leftJoin Businesses b on (p.businessId == b.id) 
    leftJoin Addresses a2 on ( b.addressId == a2.id)
    
GreenAsJade
  • 14,459
  • 11
  • 63
  • 98
  • I'd be happy to exchange brevity for readability. I've looked for ways to compose the query incrementally, but can't see how. I want to get rid of the nested tuple on the LHS, and the nested _._ on the RHS. – GreenAsJade Aug 26 '16 at 11:55

1 Answers1

5

You should experiment with something like this:

val q = Users join People joinLeft Addresses joinLeft Businesses joinLeft Addresses on {
  case ((((u, p), a), b), ba) => u.personId === p.id && p.addressId === a.flatMap(_.id) && p.businessId === b.flatMap(_.id) && b.flatMap(_.addressId) === ba.id
} map {
  case ((((u, p), a), b), ba) => (p, a, b, ba)
}

The other solution would be to make joins without using for comprehension, so you wouldn't have to use underscores to extract values from tuples:

val q = Users join People on {
  case (u, p) => u.personId === p.id
} joinLeft Addresses on {
  case ((u, p), a) => p.addressId === a.id
} joinLeft Businesses on {
  case (((u, p), a), b) => p.businessId === b.id
} joinLeft Addresses on {
  case ((((u, p), a), b), ba) => b.flatMap(_.addressId) === ba.id
} map {
  case ((((u, p), a), b), ba) => (p, a, b, ba)
}

You haven't provided full definitions of your data so I wasn't able to fully test those solutions, but this should give you some insight into a different way of defining joins in Slick. Let me know if this was helpful at all.

GreenAsJade
  • 14,459
  • 11
  • 63
  • 98
Paweł Jurczenko
  • 4,431
  • 2
  • 20
  • 24
  • Well I didn't have your data structures, so I had to guess a few things here. I assumed that after using `leftJoin` the right-hand side of the joining clause would be an `Option`. By using `.?` you're lifting a regular column to the `Option` as well, so they are comparable now. What I would suggest now is that you should remove all those `.?` and `.map` operations and try to complete the code by incrementally compiling it and trying to fix every error step-by-step. PS I have edited my post and provided another possible solution to your problem. – Paweł Jurczenko Aug 30 '16 at 10:03
  • Thanks - it's all interesting and promising: I'll see what I can make of it. – GreenAsJade Aug 30 '16 at 13:15
  • I reckon your second solution (tweaked by me - easy for me, 'cause I have the compiler :) ) is worthy of the bounty: it gets rid of the nested tuple *referencing* on the RHS. I have to say I am still hoping for something slightly more elegant ... the string of nested tuples on the LHS is still rather untidy, but it does meet my criteria of exchange brevity for understandability. I'm going to try to make the original solution work now. – GreenAsJade Aug 31 '16 at 07:12
  • Now I tweaked your first solution based on what the compiler told me. Thanks very much for your help. If you wouldn't mind commenting on why the first one needs more `flatMap()` than the second, that would be awesome. (Maybe I should ask a separate question for that?) – GreenAsJade Aug 31 '16 at 07:21
  • Interesting update: the first variant compiles, but it doesn't actually work. It returns the wrong profile! The second one does work. I wonder how they are different!? – GreenAsJade Aug 31 '16 at 07:52