Add Foldable.intercalate

[peterneyens]

Add an element between all the existing elements while folding.

[codecov-io]

Current coverage is 92.41% (diff: 100%)

Merging #1520 into master will increase coverage by <.01%

@@             master      #1520   diff @@
==========================================
  Files           246        246          
  Lines          3739       3743     +4   
  Methods        3618       3622     +4   
  Messages          0          0          
  Branches        121        121          
==========================================
+ Hits           3455       3459     +4   
  Misses          284        284          
  Partials          0          0          

Powered by Codecov. Last update 468e753...d02699e

[ceedubs]

Thanks @peterneyens. This looks good to me. The string example is the main way I could see myself using this in the real world. Out of curiosity, did you have any other use-cases in mind?

If we add this, should we add something like intercalate1 (to follow the naming convention of traverse1_ and sequence1_) to Reducible that requires only a Semigroup as opposed to a Monoid?

[peterneyens]

• Mostly as an alternative for Implement NonEmptyList#mkShowString methods #1518 (for strings) indeed.
• Another use case when I was thinking about something like intercalate was pretty printing (specifically here in fetch).
• I think @tpolecat asked once if cats had intercalate, maybe he had another use case ?

I added Reducible.intercalate1, forgot last night that we could lower the requirement to Semigroup for Reducible.

[ceedubs]

@peterneyens would you be willing to add a test for intercalate1? It'd probably be nice to hit the overridden intercalate in Reducible too.

[ceedubs]

Actually I guess the sbt-doctest examples are pretty legitimate tests. 👍 from me.

[johnynek]

It is not clear what we should expect if there is only 1 item in the foldable (nor is it tested). I would expect no additional item is added. I think that is what the code does but can you add a doc test example as well as a test?

[johnynek]

This test might be clearer if you tested against mkString. Basically implement mkString with intercalate.

Also, using a commutative monoid here (on Int) weakens the test. Using String, which is not commutative would make it stronger.

[ceedubs]

Good points @johnynek.

[tpolecat]

@peterneyens yeah I need it for generating SQL … I also did a more general thing that takes start/end brackets as well as the delimiter, which generalizes mkString and is useful but it's hard to think of a good name.

In any case 👍 for this, with test improvements as suggested above.

[adelbertc]

👍 after test improvements.

[peterneyens]

Thanks for the pointers @johnynek

• I used mkString and F[String] in the tests instead.
• I added a test for intercalate1 (it was already tested before through the overridden intercalate which is tested because ReducibleCheck extends FoldableCheck, but is probably better to be explicit).
• I added a doctest for intercalate and intercalate1 with a one item foldable. The one element foldable case is already covered by the property tests, right ?

[johnynek]

+1 on this.

[adelbertc]

Let's do this

[johnynek]

one bummer about this: using it to write mkString will be O(N^2) in cost. This is one of the problems that combineAll was designed to deal with. Not sure the best way to go here. For integers it is a waste, but for a lot of strings a huge win.

The simplest thing would be to use Foldable.toList to build a list, then intercalate only on the list, then call combineAll on the list. I guess that might be the right way to go actually.

[peterneyens]

@johnynek, so something like the following ?

def intercalate[A](fa: F[A], a: A)(implicit A: Monoid[A]): A =
  combineAll(intersperseList(toList(fa), a))

private def intersperseList[A](xs: List[A], x: A): List[A] = {
  val bld = List.newBuilder[A]
  val it = xs.iterator
  if (it.hasNext) {
    bld += it.next
    while(it.hasNext) {
      bld += x
      bld += it.next
    }
  }
  bld.result
}

// Reducible 
def intercalate1[A](fa: F[A], a: A)(implicit A: Semigroup[A]): A =
  reduce(intersperseList(toList(fa), a))

[johnynek]

Exactly.

…

On Mon, Jan 9, 2017 at 08:15 Peter Neyens ***@***.***> wrote: @johnynek <https://github.com/johnynek>, so something like the following ? def intercalate[A](fa: F[A], a: A)(implicit A: Monoid[A]): A = combineAll(intersperseList(toList(fa), a)) private def intersperseList[A](xs: List[A], x: A): List[A] = { val bld = List.newBuilder[A] val it = xs.iterator if (it.hasNext) { bld += it.next while(it.hasNext) { bld += x bld += it.next } } bld.result } // Reducible def intercalate1[A](fa: F[A], a: A)(implicit A: Semigroup[A]): A = reduce(intersperseList(toList(fa), a)) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1520 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdggEg7EnzO0mQ-maRCPnVmPQvOCeks5rQnkjgaJpZM4LcrW-> .