Added replicateA to Applicative #823

[lukewyman]

Added replicateA to Applicative. Other combinators suggested already existed by other names.

[codecov-io]

Current coverage is 89.61%

Merging #830 into master will increase coverage by +0.02% as of ebc2573

@@            master    #830   diff @@
======================================
  Files          181     181       
  Stmts         2423    2427     +4
  Branches        76      76       
  Methods          0       0       
======================================
+ Hit           2171    2175     +4
  Partial          0       0       
  Missed         252     252       

Review entire Coverage Diff as of ebc2573

Powered by Codecov. Updated on successful CI builds.

[adelbertc]

Can you add a test case for this, either as sbt-doctest or a plain test?

[lukewyman]

I don't "have the skill" for that yet. Scalacheck/property-based testing are pretty much next on my list. Should I hold off on this PR until I can get that grokked and wrapped up?

[lukewyman]

@adelbertc At this point the tests aren't making a ton of sense to me. Is there a similar test on a similar method I can look at to get the idea enough to just get that done?

[adelbertc]

@lukewyman The process will go something like this:

1. Add ApplicativeTests.scala to https://github.com/non/cats/tree/master/tests/src/test/scala/cats/tests
2. Add some tests checking for things like the result list has size equal to the Int, or anything else you may think of

An example of a test can be found https://github.com/non/cats/blob/master/tests/src/test/scala/cats/tests/FoldableTests.scala#L56

so you would have something like

class ApplicativeTests extends CatsSuite {
  test("replicateA") {
    val applicative = Applicative[Option]
    /* test here */
  }
}

[non]

Also one very small quibble with the comment on the method. fa is a F[A] value, but is itself not an applicative functor. We could say that fa has an applicative functor. This does make writing the docs a bit more awkward.

Also, I think it's worth explaining the effect of using sequence to implement this. For example, another method that has the same types would be:

def alsoReplicate[A](n: Int, fa: F[A]): F[List[A]] =
  fa.map { a => List.fill(n)(a) }

Do you all think something like this would work?

Given fa and n, apply fa n times to construct an F[List[A]] value.

[mikejcurry]

In general, given the prevalence of List in the ecosystem, it makes sense to have toList methods, but not sure about these specialisations to List in general. There is precedent in the various Foldable methods, e.g. dropWhile_, but this operation, and the Foldable operations, could equally well apply to other data structures, e.g. Stream, Streaming and I'm struggling to see what makes List important enough that it stands out.

I don't want to derail this PR, which seems fine, but just wondering is there some principle about when such specialisations to something like List should be included and when not? Or shall things just remain organic for the moment?

@non that proposed comment update seems fine to me. Difficult operation to capture the implications of succinctly, but a couple of examples would also go a long way to clarifying the effect and add a lot of value, particularly the implications if F is also a List like data structure, which may not be entirely intuitive.

scala> Applicative[List].replicateA(2, List(1, 2))
res8: List[List[Int]] = List(List(1, 1), List(1, 2), List(2, 1), List(2, 2))

scala> Applicative[Option].replicateA(2, Option(2))
res9: Option[List[Int]] = Some(List(2, 2))

[lukewyman]

@mikejcurry I'm still very much a newbie to Scala and FP in general, so this was a learning experience for me. If List is a too specific considering the more general nature of Traverse that the rest of the combinators in Applicativeuse, I'm happy to consider another approach or abandon this PR. I basically came up with the idea of identifying Monadic and Applicative combinators in "FP in Scala", and comparing that to what cats has/doesn't have, and what might be named differently.

[mikejcurry]

Don't take my comment the wrong way. I'm pretty new myself. It was just a question. :-). Same as you, just learning. :-).

I'd definitely add some sbt-doctest with examples to the scaladoc though. Some good examples in Foldable.scala here: https://github.com/ceedubs/cats/blob/1ae5d583d935d6d8bd7b2cce1d39f6a181a05ac6/core/src/main/scala/cats/Foldable.scala#L87-L104

Basically, it is where you add some code and the result that would be expected at a REPL, and the build will verify that the output is correct. This in turn ensures that there are examples right in the scaladoc for folks reading the API and ensures that the examples that are shipped compile.

[lukewyman]

@adelbertc I've added a unit test for #823. Good to go?

[ceedubs]

This might render a little cleaner in the ScalaDoc if you use backticks around fa, n, and F[List[A]].

[ceedubs]

👍 thanks!

[adelbertc]

Looks like Travis 2.10 failed, rebuilding. 👍 on green.

[ceedubs]

The build had failed again with a mysterious error. I restarted it again, but I just realized that there are now merge conflicts :(. Sorry @lukewyman would you mind updating your branch?

[lukewyman]

Hey @ceedubs, I've updated the PR so that it merges. All green :)

[ceedubs]

👍 thanks!

[lukewyman]

Hi @mikejcurry, I believe you can go ahead and close this one.