Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Read typeclass #932

Closed
pathikrit opened this issue Mar 14, 2016 · 19 comments
Closed

Add Read typeclass #932

pathikrit opened this issue Mar 14, 2016 · 19 comments

Comments

@pathikrit
Copy link

Opposite of Show:

trait Read[T] {
  def read(f: String): T
}

See also: http://eed3si9n.com/herding-cats/Read.html

@julienrf
Copy link
Contributor

I’m not sure such a class would be very useful, because the input type is fixed to String. Also, the returned value should make it possible to handle failures.

@pathikrit
Copy link
Author

Don't have any other reason besides a) Haskell has it and b) it makes sense to have "an opposite of Show" c) I need it :-)

@milessabin
Copy link
Member

It also means that we can have a law for Show, jointly with Read.

@adelbertc
Copy link
Contributor

If we move forward with this I would prefer it return Option[T]. That being said, while we can define laws for Read and Show in terms of each other..

  1. By themselves they have no laws
  2. I am not sure forAll((s: String) => read(show(s)) === Some(s)) is a useful law, especially given instances for Show show things like OneAnd(...), which is not the parsing behavior I would expect. In general it seems parsing should be done by a parser (e.g. atto)

@ceedubs
Copy link
Contributor

ceedubs commented Mar 15, 2016

I think I tend to agree with @julienrf and @adelbertc on this: I'm not yet convinced it would be a good addition to Cats. I do, in theory, like the way that it would relate to Show, but I don't think that it is something that people should actually use "in the real world".

To be honest I'm not that convinced that Show should live in Cats, but I see little harm in it being there. It's a simple type class that can be useful for teaching the type class "pattern" via log messages. However, Read would encourage being used for deserialization, which would make the exact string format produced by Show instances start to really matter. I don't think we'd ever make Cats support deserialization all that well. I think that parsing and deserialization are complex enough that they deserve libraries of their own, and great ones like atto, parboiled, jawn, circe, etc already fulfill this role.

@ceedubs
Copy link
Contributor

ceedubs commented Mar 15, 2016

One more comment: while I think Haskell is a fantastic language, I think that Cats should avoid the "because this is what Haskell does" philosophy, which I think @milessabin agrees with me on, even if we don't necessarily agree on whether Cats should gain Read :)

@milessabin
Copy link
Member

I do agree that we shouldn't do what Haskell does, just because that's what Haskell does. However, I would like to try and establish the principle that type classes can be lawful jointly and shouldn't be required to be lawful singly. Given that we don't want to remove Show and we do advocate for lawfulness I think Read would be a useful addition.

@milessabin
Copy link
Member

I agree with @adelbertc that the result type should be Option[T].

@milessabin
Copy link
Member

I'd also be happy to see both Show and Read generalized to things other than String.

@julien-truffaut
Copy link
Contributor

As far I know Show is intended to log / print something e.g. in repl but it is not a "serious" serialisation. So I would rather avoid to have a Read typeclass, one can always define a pair of typeclass for a given format e.g. Json or ByteString.

@pathikrit
Copy link
Author

pathikrit commented Mar 18, 2016

I agree on both counts that Show and Read should be generalized to things other than String and Read.read should return Option[T]. Borrowing from https://github.com/scodec/scodec a better name might be Encode and Decode:

trait Encode[F, T] {
   def encode(f: F): T
}

trait Decode[F, T] {
   def decode(f: F): Option[T]
}

type Codec[A, B] = Encode[A, B] with Decode[B, A]

Although they don't violate any laws like the other felines in https://github.com/non/alleycats, it might be a better place for commonly used versions:

type Show[A] = Encode[A, String]
type Read[A] = Decode[String, A]

type Bytes = Iterator[Byte]
type Serialize[A] = Encode[A, Bytes]
type Deserialize[A] = Decode[Bytes, A]

@sir-wabbit
Copy link

type Codec[A, B] = Encode[A, B] with Decode[B, A]
// is the same as
type Codec[A, B] = (A => B, B => Option[A])

And that looks a lot like a prism. And prisms (and all other optics) have laws, form categories, splits, etc. As far as I understand, Read[A] and Show[A] could be collapsed into a lawful type class:

final case class Prism[A, B](encode: A => B, decode: Kleisli[Option, B, A])
// It could be trait Prism[A, B] {...}
// Laws:
// decode(encode(x)) = Some(x)
// decode(x).map(encode) = Some(x) or None
//
// It is also a category, a split (almost an arrow), 
// an invariant functor in both type parameters, 
// maybe even a cartesian in both arguments (assuming that SemigroupK[Option] is defined) (?).

object Prism {
  def id[A]: Codec[A, A] = Prism(identity, Kleisli(Some.apply))

  implicit val instance = new Category[Prism] with Split[Prism] {
    override def id[A]: Prism[A, A] = Prism.id[A]
    override def compose[A, B, C](f: Prism[B, C], g: Prism[A, B]): Prism[A, C] =
      Prism(
        encode=f.encode compose g.encode,
        decode=f.decode andThen g.decode)
    override def split[A, B, C, D](f: Prism[A, B], g: Prism[C, D]): Prism[(A, C), (B, D)] =
      Prism[(A, C), (B, D)](
        encode=(a: A, c: C) => (f.encode(a), g.encode(c)),
        decode=Split[Kleisli[Option, ?, ?]].split(f.decode, g.decode))
  }

  implicit def leftInvariant[X]: Invariant[Prism[?, X]] = ???
  implicit def rightInvariant[X]: Invariant[Prism[X, ?]] = ???
}

def println[A](x: A)(implicit codec: Prism[A, String]): IO[Unit] = ???

I personally don't see much of a point in having Show[A] considering that there is a _.toString method on every class and Show[A] is intended for debugging purposes. I think that both Show[A] and Read[A] (if implemented) will be abused.

@EncodePanda
Copy link

Just quick thought: If you are going to implement Read typeclass, please don't call it Decode :) It might be just confusing for anyone coming from ScalaZ, Haskell etc.

@OlivierBlanvillain
Copy link
Contributor

I think cats should adopt the same position than the one with Task and recommend other libraries instead of adding a Read typeclass.

@pathikrit
Copy link
Author

@OlivierBlanvillain : What other libraries would you recommend for Read typeclass?

@adelbertc
Copy link
Contributor

@pathikrit Read is generally used as a quick way to do some parsing from a String into a type, so I think you should be looking at proper parsing libraries as an alternative. These come to mind:

@johnynek
Copy link
Contributor

Yeah, I’m -1 on Read.

@tpolecat
Copy link
Member

tpolecat commented Oct 16, 2017

I'm also 👎 . As a practical matter it's not useful because it doesn't compose unless you build it out into a parser combinator library (of which there are already many to choose from). And as an dual to Show it's also not very useful because Show itself is not very useful; it's no more than a properly parametric toString and is similarly useful only for debugging. I think the key insight is that the string representation isn't uniquely determined by the type, so what we're talking about here is not a typeclass. A better representation would be something like a printing/parsing pair joined as a Prism or similar, as normal data.

@ceedubs
Copy link
Contributor

ceedubs commented Mar 25, 2018

While there are various points of view on this, it seems that the majority of Cats maintainers don't think that Read belongs in Cats. For real-world use-cases you probably want more than an Option as a result, as you probably want to know a reason that a String failed to be parsed. I'd recommend a parsing library such as @adelbertc recommended here.

This issue has been hanging around for quite some time. In an effort to clean up some of the stale backlog, I'm going to go ahead and close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests