product and map can be implemented in terms of ap, fixes #904

[adelbertc]

No description provided.

[codecov-io]

Current coverage is 90.91%

Merging #967 into master will decrease coverage by -0.07% as of 3fc3080

@@            master    #967   diff @@
======================================
  Files          181     181       
  Stmts         2153    2157     +4
  Branches        42      42       
  Methods          0       0       
======================================
+ Hit           1959    1961     +2
  Partial          0       0       
- Missed         194     196     +2

Review entire Coverage Diff as of 3fc3080

Powered by Codecov. Updated on successful CI builds.

[travisbrown]

I don't know how much this should matter, but I just set up a simple benchmark with two traits extending Applicative, one with a default ap and one with a default map and product, and three implementations of Applicative[Option] in terms of these traits:

• instance1: default map and product, only ap defined.
• instance2: default map and product, but overridden.
• instance3: default ap.

And the results for sequencing a list of options look like this:

[info] Benchmark                        Mode  Cnt      Score     Error  Units
[info] ApplicativeBenchmark.traverse1  thrpt   40  13317.254 ±  44.992  ops/s
[info] ApplicativeBenchmark.traverse2  thrpt   40  21190.597 ± 165.308  ops/s
[info] ApplicativeBenchmark.traverse3  thrpt   40  21327.822 ±  69.654  ops/s

I.e., with default map and product, doing the naive thing and only implementing what scalac tells you to implement is an instant 40% tax on your throughput for at least one common use case.

This is probably more an argument for moving all (or most) default implementations into pedagogical subtypes as soon as possible, since I agree that going in the direction you've chosen here makes the most sense pedagogically.

[adelbertc]

I'm a stickler for consistency which is mainly what's driving this PR. If we can get the whole default thing sorted out soon then I'm happy to close the PR in favor of that :-)

[ceedubs]

I agree with @adelbertc that this makes sense from a consistency perspective.

I agree with @travisbrown that this is probably never the implementation that you want in terms of performance and that we should move default implementations into subtypes very soon.

Overall, 👍 from me, because people can always override the default implementation for now if they don't want it, and we have said that we will move default implementations for the 1.0 release.

[travisbrown]

👍 from me as well.