arrow icon indicating copy to clipboard operation
arrow copied to clipboard

Ior mapOrAccumulate

Open sampengilly opened this issue 1 year ago • 2 comments

Add support for mapOrAccumulate functions which when run inside an IorRaise scope accumulate errors into a Both case with the combined errors and any successfully mapped items.

This change adds the basic mapOrAccumulate functions, it doesn't provide any additional convenience functions like those which take an Iterable<T> receiver such as those found in Iterable.kt. As I see it, in order to provide those functions we'd either need to wait for context parameters, or move each of those existing functions into the Raise interface and define the new ones in the IorRaise class.

I also haven't included the zipOrAccumulate functions in this change. I intend to do those as it feels like a use case which makes sense (carry out N operations, collect the errors, but if some succeeded then treat it as a success overall).

sampengilly avatar Jul 17 '24 09:07 sampengilly

Looking at it again. I don't think that zipOrAccumulate makes sense for IorRaise as it's essentially an applicative, the final builder block only makes sense if all of the inputs have succeeded, there doesn't seem to be any room for a partial success that can be represented by an Ior in that case.

sampengilly avatar Jul 26 '24 11:07 sampengilly

Is there any overlap here with #3436? That PR looks like it provides a better general purpose API that is much more DSL like, however I feel like this PR is more focused on the specific case of mapping over iterables and accumulating the errors.

Would the list case (such as parsing a collection of items, dropping failed ones and accumulating errors for them) be satisfied in the DSL approach with something like this?

fun Raise<Error>.parseItem(item: SomeItemType): ParsedType = ...

iorNel {
  val originalList = listOf<SomeItemType>(...)
  val parsedList by accumulating { originalList.map(::parseItem) }
  parsedList
} shouldBe Ior.Both(...)

EDIT: Actually that couldn't work on its own. There'd need to be an additional layer catching the raised error from parseItem and determining what to do with that item in the overall map operation

sampengilly avatar Sep 10 '24 11:09 sampengilly

@kyay10 does your PR #3665 supersede this one?

serras avatar Jul 23 '25 19:07 serras

#3665 indeed supercedes this! In fact, this might've been my inspiration in the first place, but I can't remember

kyay10 avatar Jul 24 '25 02:07 kyay10

Yea, this PR pre-dates the accumulate DSL PR... so I figured that superseded the API I was presenting here. If there's a new attempt at providing these utilities on top of the new API that's good to see :)

sampengilly avatar Jul 24 '25 10:07 sampengilly

Closed because #3665 fixed this. Please reopen (or ideally make a new PR) if the new APIs aren't ideal!

kyay10 avatar Jul 25 '25 18:07 kyay10