I'm struggling with figuring out how to properly c...

# arrow

[CLOVIS]

I'm struggling with figuring out how to properly compose the either DSL and EitherNel.

I have a recursive n-ary tree called Field. Each node has a validation method. If a node is invalid, all its parents are invalid. Two sibling/cousin nodes are independent (one being invalid does not affect the other). For UX reasons, I'd like to bubble up as many independent errors as possible, so the user can see whether multiple nodes are invalid.

I have a sealed class which represents the reasons a node may be invalid. Let's call it Compatibility . The signature is therefore:

sealed class Field {
    val children: Map<Int, Field>
    …
    suspend fun validate(): EitherNel<Compatibility, Unit>
}

Each type of node has different logic for validation. Most of these are simple (Either<Compatibility, Unit>), but of course, a node must check that all its children are invalid.

Let's take the example of a node `C`:
• it must first check the condition a() (because of a kotlin contract which refines a value); otherwise it should fail with Compatibility.A()
• for each of its direct children, it should check the condition b() ; otherwise it should fail with Compatibility.B()
• for each of its direct children, it should recursively call itself, and bubble up whatever errors were caused
For the last two, failures of two different children are independent, therefore we want to return them both.

Currently, I managed to write this:

class C : Field {
    …

    override suspend fun validate(): EitherNel<Compatibility, Unit> {
        if (a()) {
            Compatibility.A()
                .left()
                .mapLeft { nonEmptyListOf(it) }
        }

        return children.map { (childId, child) ->
            if (b()) {
                listOf(Compatibility.B())
            } else {
                child.validate()
                    .leftOrNull()
                    ?: emptyList()
            }
        }.flatten()
            .toNonEmptyListOrNull()
            ?.left()
            ?: Unit.right()
    }
}

This feels very verbose, I feel like I'm missing something. Especially because of how it would look without the Nel :

class C : Field {
    …

    override suspend fun validate() = either {
        ensure(a()) { Compatibility.A() }

        for ((childId, child) in children) {
            ensure(b()) { Compatibility.B() }
            child.validate().bind()
        }
    }
}

What is the correct way to structure this to be easier to read?

[simon.vergauwen]

I think you’d want to use zipOrAccumulate for the two independent ensure and map. But instead of map use mapOrAccumulate inside of the zipOrAccumulate. I can write a snippet for you in the morning.

[CLOVIS]

Thanks, I don't really see it at the moment

[simon.vergauwen]

An example based on the code you shared.

import arrow.core.EitherNel
import arrow.core.raise.either
import arrow.core.raise.ensure
import arrow.core.raise.zipOrAccumulate

sealed interface Compatibility {
  data class A(val a: String = "a") : Compatibility
  data class B(val b: String = "b") : Compatibility
}

fun a(): Boolean = TODO()
fun b(): Boolean = TODO()

class Field {
  private val children: List<Field> = emptyList()

  suspend fun validate(): EitherNel<Compatibility, Unit> =
    either {
      zipOrAccumulate(
        { ensure(a()) { Compatibility.A() } },
        {
          children.mapOrAccumulate {
            ensure(b()) { Compatibility.B() }
          }
        }) { _, _ -> }
    }
}

[CLOVIS]

I see. That's quite hard to read, unfortunately...

[simon.vergauwen]

You can split it up though

[CLOVIS]

Created https://github.com/arrow-kt/arrow/issues/3048, which could be one improvement to this example

[CLOVIS]

Here's what the real code looks like after your improvements:

[mitch]

@CLOVIS what if we use the extension function instead? T.nel(): NonEmptyList<T> would that improve readability?

something like

ensure(...) {
  invalidType(...).nel()
}

Oh by the way.. Just my opinion, I think I would prefer to make Raise<E>
ensure / bind / raise only operate on the E type.. If we add another bind for the inner E in Nel<E>, imo that would make it very confusing for early adopters…

RaiseNel<E> type alias would be really nice btw.

[CLOVIS]

Maybe name it ensureSingle or something similar, so it's not an overload and thus less confusing?