enhancement to build ReadWriter classes by composition

[ekk-cb]

I occasionally need to implement my own ReadWriter[X]. So far I've always been able to do this by converting to some other type that already has an instance. This requires two functions, one to map to that type and one to map back from it. This is common enough that I've made a an implicit class to add syntax.

object Helper {
  implicit class EnhancedReadWriter[A](val _rw: ReadWriter[A]) extends AnyVal {
    def compose[B](down: B => A, up: A => B): ReadWriter[B] = new ReadWriter[B](
      write = a => _rw.write(down(a)),
      read = { case (b) => up(_rw.read(b)) })
  }
}

I use it like this:

implicit val dateReadWrite: ReadWriter[Date] = (implicitly[ReadWriter[Long]]).compose(
      down = _.getTime,
      up = new Date(_) )

Is this worth adding centrally?

ID: 7 Original Author: drdozer ID: 6 Original Author: ekk-cb

[ekk-cb]

Seems reasonable; just some small nits over the API. Does it really need to be an extension method? I'm imagining something like

implicit val dateRW: ReadWriter[Date] = upickle.composeRW[Long](
      down = _.getTime,
      up = new Date(_) 
)

Though I'm not sure if type-inference and everything lines up right.

Original Author:lihaoyi Original Author:ekk-cb

[ekk-cb]

My rule of thumb is to do everything outside of the core operations exposed via a type as extension methods, unless the type is a concrete class. It's a style thing in many cases, but sometimes there are no good alternative choices. Here, ReadWriter is defined as the union of the Read and Writer traits, so I didn't see a good class or trait to host this method directly. Also, my presumption (possibly wrong) is that this style potentially enables much more aggressive inlining strategies.

Original Author:drdozer Original Author:ekk-cb

[ekk-cb]

It would be pretty trivial to create a ReadWriter "companion", do what I did and just make it a top-level; I feel that unless we really want the infix property (e.g. for APIs you'll use lots and lots in a small space) normal static-methods should be the default. In particular, pulling out something using implicitly just to call an extension method on it is kinda weird =P

If we're doing this we should have separate composeX functionality for Reader and Writer too, just because the entire library separates these two concepts since you may not need/want/be-able-to do both, and macro-materializing both when you only need one is wasteful.

I don't know anything about inlining and wouldn't worry to much about that for now =)

Original Author:lihaoyi Original Author:ekk-cb

[ekk-cb]

In that case, these individual operations are map and comap on the reader and writer, and a read-writer is really a new type that is a pair of a reader and a writer, not their intersection. However, we're probably over-thinking the types at this point and are in danger of re-inventing scalaz ;)

It's the functionality I need, not any particular API (infix or otherwise). I'd be fine with:

ReadWriter.compose[A, B](rw: ReadWriter[A], A => B, B => A): ReadWriter[B] = ...

I just need something that hides this transformation away.

Original Author:drdozer Original Author:ekk-cb

[ekk-cb]

Closing this for now just to clean house... Feel free to send a PR if this still matters to you =)

Original Author:lihaoyi Original Author:ekk-cb