Skip to content

fix the "Factory" type alias definition #152

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

Merged
merged 2 commits into from
Feb 4, 2019
Merged

Conversation

xuwei-k
Copy link
Contributor

@xuwei-k xuwei-k commented Sep 12, 2018

avoid import scala.collection.compat._ everywhere in Scala 2.12 and 2.11

collection-compat v0.2.0

Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
<console>:12: error: could not find implicit value for parameter e: collection.compat.Factory[Int,List[Int]]
       implicitly[scala.collection.compat.Factory[Int, List[Int]]]
                 ^

scala> import scala.collection.compat._
import scala.collection.compat._

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
res1: collection.compat.Factory[Int,List[Int]] = scala.collection.generic.GenTraversableFactory$$anon$1@41b12185

collection-compat v0.1.1

Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
res0: scala.collection.compat.Factory[Int,List[Int]] = scala.collection.compat.Factory$$anon$1@33600943

avoid "import scala.collection.compat._" everywhere in Scala 2.11 and 2.12

collection-compat v0.2.0

```
Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
<console>:12: error: could not find implicit value for parameter e: collection.compat.Factory[Int,List[Int]]
       implicitly[scala.collection.compat.Factory[Int, List[Int]]]
                 ^

scala> import scala.collection.compat._
import scala.collection.compat._

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
res1: collection.compat.Factory[Int,List[Int]] = scala.collection.generic.GenTraversableFactory$$anon$1@41b12185
```

collection-compat v0.1.1

```
Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_181).
Type in expressions for evaluation. Or try :help.

scala> implicitly[scala.collection.compat.Factory[Int, List[Int]]]
res0: scala.collection.compat.Factory[Int,List[Int]] = scala.collection.compat.Factory$$anon$1@33600943
```
@MasseGuillaume
Copy link
Contributor

LGTM, Can you look if it fixes #137.

@julienrf julienrf requested a review from smarter September 12, 2018 19:02
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

If we go this far then to be consistent we should remove the implicit conversion from CanBuildFrom to Factory, and maybe other conversions too.

@julienrf
Copy link
Contributor

julienrf commented Nov 8, 2018

I think the reported problem is not too important, and I do have a preference towards using <:, which meaning is closer to opaque types than =, AFAIU. I’m closing the PR but feel free to re-open it and debate it.

@julienrf julienrf closed this Nov 8, 2018
@julienrf
Copy link
Contributor

julienrf commented Jan 26, 2019

After thinking more about it, maybe using a type alias (as proposed in this PR) is better than using <:.

Indeed, with <:, users have to add the import scala.collection.compat._ to get their CanBuildFrom instance converted to the expected Factory existential type. This means that libraries that use Factory will require their users to write this additional import, whereas with a type alias that import wouldn’t be necessary.

Do other people have any thoughts about merging this PR?

@julienrf julienrf reopened this Feb 4, 2019
Since Factory is now a transparent type alias to CanBuildFrom, we
don’t need anymore conversions from CanBuildFrom to Factory.
@julienrf julienrf merged commit f4ebf53 into scala:master Feb 4, 2019
@julienrf
Copy link
Contributor

julienrf commented Feb 4, 2019

Thanks @xuwei-k !

@xuwei-k xuwei-k deleted the Factory-type branch April 30, 2020 05:01
martijnhoekstra pushed a commit to martijnhoekstra/scala-collection-compat that referenced this pull request Nov 9, 2022
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

Successfully merging this pull request may close these issues.

4 participants