Batching method implementation - Scala

Question

I need to implement a function/method that operates on a sequence of values, and acts kind of like takeWhile, but with 2 differences, first, it doesn't act on a single element, it is performed on a sequence of elements, second, it won't just return after the predicate is satisfied for the first time, it will keep going until all those values inside the main sequence are grouped together. I might've not explained it very well, so let me provide an example. I have these values inside the sequence:

val seq = Seq(1, 4, 2, 4, 2, 5, 6, 7, 1, 9, 2, 3, 4)

And I want to group these values inside some inner sequences, that the sum of all the values inside inner sequences is less than or equal to 10:

val result: Seq[Seq[Int]] = Seq(Seq(1, 4, 2), Seq(4, 2), Seq(5), Seq(6), Seq(7), Seq(1, 9), Seq(2, 3, 4))

Notes:

• The order does not matter
• Each sequence should be as large as it can be, for instance, first element of the result could've been Seq(1, 4), or Seq(1), but I want it to be the biggest number possible.

I have 2 different implementations, I wanted to know which one is better? Based on performance, readability or anything else. If anyone also has a better approach, I will be glad to see.

implicit class SeqOps[T](seq: Seq[T]) {

    // approach #1
    def batchUntil(predicate: Seq[T] => Boolean): Seq[Seq[T]] =
      seq.foldLeft[Seq[Seq[T]]](z = Nil)(
        (acc, currentElem) => acc.headOption.collect {
          case currentAggregator if predicate(currentElem +: currentAggregator) =>
            (currentElem +: currentAggregator) +: acc.tail
          case _ =>
            (currentElem :: Nil) +: acc
        }.getOrElse(Seq(currentElem :: Nil))
      )

    // END of approach #1
    // Approach #2

    def partitionedUnordered(predicate: Seq[T] => Boolean): Seq[Seq[T]] = partitioningHelper(seq, Nil, Nil)(predicate)

    @tailrec
    private def partitioningHelper(remainder: Seq[T], acc: Seq[Seq[T]], currentAggregator: Seq[T])(predicate: Seq[T] => Boolean): Seq[Seq[T]] = {
      if (remainder.isEmpty) {
        if (currentAggregator.isEmpty) acc
        else currentAggregator +: acc
      }
      else {
        val newAgg = remainder.head +: currentAggregator
        if (predicate(newAgg)) partitioningHelper(remainder.tail, acc, newAgg)(predicate)
        else partitioningHelper(remainder.tail, currentAggregator +: acc, remainder.head +: Nil)(predicate)
      }
    }
  }

And if you're wondering about why on earth a human being should do this, is that I need to chunk some load of data into small JSON arrays that each are smaller than 1MB, so imagine an array of Json objects (total 100MB), I need to batch it like:

Seq([first array 970KB], [second array 995KB], ...)

Answer 1

Both implementations are perfectly reasonable, but they don't always produce the same result.

Seq(9).batchUntil(_.sum < 0)           //Seq(Seq(9))
Seq(9).partitionedUnordered(_.sum < 0) //Seq(Seq(9), Seq())

I'm not a fan of long lines, i.e. horizontal code, and I find the nested if...else in the @tailrec approach (#2) rather difficult to parse and reason about.

The foldLeft() approach (#1) can be more concise and, I think, made clearer by utilizing the power of pattern matching.

def batchUntil(predicate: Seq[T] => Boolean): Seq[Seq[T]] =
  seq.foldLeft[Seq[Seq[T]]](Seq()) {
    case (acc@currentAggregator+:remainder, currentElem) =>
      val newAgg = currentElem +: currentAggregator
      if (predicate(newAgg))     newAgg +: remainder
      else                       Seq(currentElem) +: acc
    case (Seq(), currentElem) => Seq(Seq(currentElem))
  }

You'll note that I don't use :: or Nil. Your code indicates some confusion between Seq operations like +:, and List operations using :: and Nil.

Seq is a trait so it's not really a concrete collection type, more like an interface of methods common to multiple real collections such as List and Vector.

Methods such as batchUntil() might take a passed parameter of type Seq[T] to indicate that the calling code can pass any concrete collection type as long as it's a member of the Seq family. Returning a Seq, on the other hand, is seldom advisable. It hides the underlying concrete collection type from the receiver.

It is possible to take multiple collection types and return the same type as received, but that's a bit beyond the scope of this code review.