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.

Sign up for GitHub

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

Jump to bottom

#607: Add missing Show instances #928

Merged
merged 3 commits into from
Mar 16, 2016
Merged

#607: Add missing Show instances #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4dae55b
Initial commit for ISSUE-607. Wondering if typeclass instances for S…
Mar 11, 2016
ba6b93d
Removing superfluous comment.
Mar 11, 2016
42a0583
Simplifying TupleTests
Mar 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/src/main/scala/cats/data/WriterT.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ final case class WriterT[F[_], L, V](run: F[(L, V)]) {

def reset(implicit monoidL: Monoid[L], functorF: Functor[F]): WriterT[F, L, V] =
mapWritten(_ => monoidL.empty)

def show(implicit F: Show[F[(L, V)]]): String = F.show(run)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm we should be able to delete this line right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adelbertc isn't the Show instance delegating to this method?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the Show instance is delegating to this method. I modeled this on the change that added a Show instance for OptionT (see: https://github.com/typelevel/cats/pull/600/files; and in particular:

cats/core/src/main/scala/cats/data/OptionT.scala

Line 90 in 6dbd4b0

def show(implicit F: Show[F[Option[A]]]): String = F.show(value)
). But I agree it is not strictly needed (assuming the Show instance is updated accordingly).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see now, I read it as Show[(L, V)] - ignore me :-)

}
object WriterT extends WriterTInstances with WriterTFunctions

Expand All @@ -68,6 +70,10 @@ private[data] sealed abstract class WriterTInstances extends WriterTInstances0 {
def liftT[A](ma: M[A]): WriterT[M, W, A] =
WriterT(M.map(ma)((W.empty, _)))
}

implicit def writerTShow[F[_], L, V](implicit F: Show[F[(L, V)]]): Show[WriterT[F, L, V]] = new Show[WriterT[F, L, V]] {
override def show(f: WriterT[F, L, V]): String = f.show
}
}

private[data] sealed abstract class WriterTInstances0 extends WriterTInstances1 {
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/scala/cats/std/tuple.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,10 @@ sealed trait Tuple2Instances {
def bifoldRight[A, B, C](fab: (A, B), c: Eval[C])(f: (A, Eval[C]) => Eval[C], g: (B, Eval[C]) => Eval[C]): Eval[C] =
g(fab._2, f(fab._1, c))
}

implicit def tuple2Show[A, B](implicit aShow: Show[A], bShow: Show[B]): Show[(A, B)] = new Show[(A, B)] {
override def show(f: (A, B)): String = {
s"(${aShow.show(f._1)},${bShow.show(f._2)})"
}
}
}
23 changes: 23 additions & 0 deletions tests/src/test/scala/cats/tests/TupleTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,27 @@ import cats.laws.discipline.eq.tuple2Eq
class TupleTests extends CatsSuite {
checkAll("Tuple2", BitraverseTests[Tuple2].bitraverse[Option, Int, Int, Int, String, String, String])
checkAll("Bitraverse[Tuple2]", SerializableTests.serializable(Bitraverse[Tuple2]))

test("show") {
(1, 2).show should === ("(1,2)")

forAll { fs: (String, String) =>
fs.show should === (fs.toString)
}

// Provide some "non-standard" Show instances to make sure the tuple2 is actually use the Show instances for the
// relevant types instead of blindly calling toString
case class Foo(x: Int)
implicit val fooShow: Show[Foo] = new Show[Foo] {
override def show(f: Foo): String = s"foo.x = ${f.x}"
}
case class Bar(y: Int)
implicit val barShow: Show[Bar] = new Show[Bar] {
override def show(f: Bar): String = s"bar.y = ${f.y}"
}

val foo = Foo(1)
val bar = Bar(2)
(foo, bar).show should === (s"(${fooShow.show(foo)},${barShow.show(bar)})")
}
}
5 changes: 5 additions & 0 deletions tests/src/test/scala/cats/tests/WriterTTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ class WriterTTests extends CatsSuite {
}
}

test("show") {
val writerT: WriterT[Id, List[String], String] = WriterT.put("foo")(List("Some log message"))
writerT.show should === ("(List(Some log message),foo)")
}

{
// F has a SemigroupK
implicit val F: SemigroupK[ListWrapper] = ListWrapper.semigroupK
Expand Down