Utility functions for `RenderContext`

[serras]

These are a few utilities I end up using over and over in my projects. For example, if I have nullable fields, the following is a bit cumbersome:

thing.data.render { t ->
  if (t != null) { 
    // ... keep going ...
  }
}

hence the utility for

thing.data.renderIfNotNull { t ->
  // ... keep going ...
}

The same happens for empty lists, where one usually needs to consider the empty list case differently (for example, showing No results found), but which right now requires nested render. My suggestion would turn this into:

results.data.renderEachIfNotEmpty(
  otherwise = { + "No results" }
) {
  // render one element
}

[Lysander]

Hello and first of all thank you for your PR and the fact you "use" fritz2!

Your proposal has some valid aspects, but honestly I am not a fan of APIs that manage all with lambdas ;-)

So let us discuss a bit about the typical use cases of boilerplate elimination, which is the main aspect of your proposals.

It is in indeed very common to have nullable flows and only wants to render in the none null case:

// some nullable data store
storedThing.data.render { thing ->
    if(thing != null) {
        // arbitrary complex UI code
    }
}

// your proposal
storedThing.data.renderIfNotNull { thing ->
    // arbitrary complex UI code
}

Imho really a common use case we have encountered also in many projects yet. Question: Do we need the if in the name? common flow and collection functions omit the if like mapNotNull for example. So renderNotNull could be sufficient?

What I honestly really do not like in general are all those otherwise-lambdas! I do not see the benefit of moving the "else" case in front of the regular rendering code. Also multiple lambdas - even with some complex decalartive UI nesting - inside a function call are often harder to read, than a simple if ... else-block inside some render-function:

// Is this really better readable?
storedThing.data.renderIfNotNull({
    div {
        span { svg { ... } }
        span { +"No Thing there..." }
    }
}) { thing ->
    // arbitrary complex UI code
}

// than this:
storedThing.data.render { thing ->
    if(thing != null) {
        // arbitrary complex UI code for postive case
    else {
        div {
            span { svg { ... } }
            span { +"No Thing there..." }
        }
    }
}

In my opinion the explicit if-else fits better into the declarative UI approach of fritz2.

So that's also why I think the renderEachIfNotEmpty would not add much benefit, as you said the common pattern is to deal with both cases, which you can easily do with some if-else branching.

The renderIf is the function, where I am uncertain about. Without the otherwise parameter this could indeed easier to read.

But I could imagine two explicit variants as extension on Flow<Boolean> like renderTrue and renderFalse, which will definitely save boilerplate for boolean flows.

I think the overall approach is more or less personal "taste" and could not be judged without bias. As I have recognized that you are a fan of FP, it is clear to me, why you like configurable functions by lambdas ;-)

Nevertheless there are multiple ways to progress with this PR. I would appreciate to hear about your opinion about my points.

To summarize my pov:

• renderNotNull -> definitely good thing!
• renderIf -> could make sense
• renderTrue / renderFalse -> could also make sense. (I will try to check some internal projects for practical use cases and add those here)
• renderEachIfNotEmpty -> I am not convinced that this is helpful
• drop otherwise in any case

If we evolve the original PR in a way that the whole team agrees to include into the fritz2 core, we definitely need

• unit tests
• description / sub-section or only appropriate "mentining" inside our documentation

We also always appreciate to feature fritz2 related projects. So if you prefer to provide those functions as they are, feel free to host your own "utils-fritz2"-project, which we could also mention in our docs or even with some blog-post

[serras]

About the naming: the reason why I chose renderIfNotNull was because renderNotNull would seem to follow the pattern of mapNotNull in which the billable is in the result position (instead of argument). renderIf is closer to takeIf in the standard library.

[serras]

About renderIfNotEmpty, the pattern I was trying to abstract there was the requirement for render you check the size, followed by renderEach. This looks very cumbersome to me.

But maybe the solution is to “upgrade” to Myers algorithm to take emptiness of collections into account?

[serras]

Agreed about otherwise. I mostly added it because it was easy to add.

[serras]

I've given this another go, removing the otherwise parameter and leaving only renderIf and renderNotNull. I've realized that the non-empty could be very simply rewritten as

renderIf(List<*>::isNotEmpty) {
  // only do this when there's some element
}

I've also realized that there was a missing function to render only when the value satisfies some type. This is especially useful if you're defining states as a sealed interface, so you can write:

renderIs(Loading::class) {
  + "Loading"
}
renderIs(Found::class) {
  + "Found ${it.size} elements"
}

Of course, the best syntax would be instead to have:

renderIs<Loading> {
  + "Loading"
}

but we hit the wall of the Kotlin compiler: (1) we cannot define the type argument as reified because RenderContext is an interface, but (2) we cannot define it as an extension method because it has two receiver.

[serras]

To complete the entire set of way to match values in Kotlin, we could add a final:

fun <E: Enum<E>> Flow<E>.matchValue(x: E, content: Tag<*>.() -> Unit)

so you could write:

store.isAdult.data.matchValue(true) {
  // do only when something is true
}
store.weather.data.matchValue(CLOUDY) {
  + "This is a gray day"
}

[Lysander]

Triggered by your PR a colleague and I played a little to achieve a way to write this one:

someFlow.renderIfNotNull { thing ->
    // ui code if thing is not null
}.orElse { // we need a "special" return type for those `render`-variants with possible `orElse` continuation!
    // ui code if thing is null
}

This would be a much better readable approach to the otherwise parameter and could be the solution to improve the rendering of list based flows, dealing with the empty corner case.

That said, it is not as trvial as we thought at first. I hope we find the time to think again about this, as I kinda like the idea to be honest!

What do you think about the syntax sketch?

I appreciate the new proposals - just need some time to review those... (Maybe during the weekend!)

[serras]

Although for small examples this seems quite nice, it seems to me that any solution comes to a problem when things are nested. Imagine I have the following:

someFlow.renderIfNotNull { some ->
  otherFlow.renderIfNotNull { other ->
    // this only works when both are not null
  }
}.orElse { ... }

What are the semantics we want here? It might be useful to make it work such as if any of the tests fail, then the orElse part is executed. But in order to do so the "outcome" of otherFlow.renderIfNotNull should be "visible" to the outer layer.

[jamowei]

I personally like the three new convenience methods you create! Thanks for your effort 👍🏻

[Lysander]

I have to agree - looks nice now!

So there is some work left as I have already teasered in my first comment:

• we need unit tests for the new render-methods
• we need to integrate those new features into the documentation

Do you want to add those things in your PR? Or should we merge your PR first and then add those aspects by our own in a new PR we manage in fritz2?