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 Compose instance for Map #2402

Merged
merged 4 commits into from
Aug 16, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions core/src/main/scala/cats/implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ object implicits
with syntax.AllSyntaxBinCompat2
with instances.AllInstances
with instances.AllInstancesBinCompat0
with instances.AllInstancesBinCompat1
3 changes: 3 additions & 0 deletions core/src/main/scala/cats/instances/all.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,6 @@ trait AllInstances
trait AllInstancesBinCompat0
extends FunctionInstancesBinCompat0
with Tuple2InstancesBinCompat0

trait AllInstancesBinCompat1
extends MapInstancesBinCompat0
20 changes: 20 additions & 0 deletions core/src/main/scala/cats/instances/map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package instances
import cats.kernel.CommutativeMonoid

import scala.annotation.tailrec
import cats.arrow.Compose

trait MapInstances extends cats.kernel.instances.MapInstances {

Expand Down Expand Up @@ -79,4 +80,23 @@ trait MapInstances extends cats.kernel.instances.MapInstances {

}
// scalastyle:on method.length

}

trait MapInstancesBinCompat0 {

implicit def catsStdComposeForMap: Compose[Map] = new Compose[Map] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a val instead of a def so the same instance can be reused repeatedly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


def compose[A, B, C](f: Map[B, C], g: Map[A, B]): Map[A, C] = {
g.foldLeft(Map.empty[A, C]) {
case (acc, (key, value)) =>
f.get(value) match {
case Some(other) => acc + (key -> other)
case _ => acc
}
}
}

}

}
4 changes: 2 additions & 2 deletions core/src/main/scala/cats/instances/package.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cats

package object instances {
object all extends AllInstances with AllInstancesBinCompat0
object all extends AllInstances with AllInstancesBinCompat0 with AllInstancesBinCompat1
object bigInt extends BigIntInstances
object bigDecimal extends BigDecimalInstances
object bitSet extends BitSetInstances
Expand All @@ -21,7 +21,7 @@ package object instances {
object invariant extends InvariantMonoidalInstances
object list extends ListInstances
object long extends LongInstances
object map extends MapInstances
object map extends MapInstances with MapInstancesBinCompat0
object option extends OptionInstances
object order extends OrderInstances
object ordering extends OrderingInstances
Expand Down
4 changes: 2 additions & 2 deletions testkit/src/main/scala/cats/tests/CatsSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package cats
package tests

import catalysts.Platform
import cats.instances.{AllInstances, AllInstancesBinCompat0}
import cats.instances.{AllInstances, AllInstancesBinCompat0, AllInstancesBinCompat1}
import cats.syntax.{AllSyntax, AllSyntaxBinCompat0, AllSyntaxBinCompat1, AllSyntaxBinCompat2, EqOps}
import org.scalactic.anyvals.{PosInt, PosZDouble, PosZInt}
import org.scalatest.{FunSuite, FunSuiteLike, Matchers}
Expand Down Expand Up @@ -33,7 +33,7 @@ trait CatsSuite extends FunSuite
with GeneratorDrivenPropertyChecks
with Discipline
with TestSettings
with AllInstances with AllInstancesBinCompat0
with AllInstances with AllInstancesBinCompat0 with AllInstancesBinCompat1
with AllSyntax with AllSyntaxBinCompat0 with AllSyntaxBinCompat1
with AllSyntaxBinCompat2
with StrictCatsEquality { self: FunSuiteLike =>
Expand Down
4 changes: 3 additions & 1 deletion tests/src/test/scala/cats/tests/MapSuite.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cats
package tests

import cats.laws.discipline.{FlatMapTests, SemigroupalTests, SerializableTests, UnorderedTraverseTests}
import cats.laws.discipline.{FlatMapTests, SemigroupalTests, SerializableTests, UnorderedTraverseTests, ComposeTests}

class MapSuite extends CatsSuite {
implicit val iso = SemigroupalTests.Isomorphisms.invariant[Map[Int, ?]]
Expand All @@ -15,6 +15,8 @@ class MapSuite extends CatsSuite {
checkAll("Map[Int, Int] with Option", UnorderedTraverseTests[Map[Int, ?]].unorderedTraverse[Int, Int, Int, Option, Option])
checkAll("UnorderedTraverse[Map[Int, ?]]", SerializableTests.serializable(UnorderedTraverse[Map[Int, ?]]))

checkAll("Compose[Map]", ComposeTests[Map].compose[Int, Long, String, Double])
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, can we add the SerializableTests as the other instances here?



test("show isn't empty and is formatted as expected") {
forAll { (map: Map[Int, String]) =>
Expand Down