fritz2 icon indicating copy to clipboard operation
fritz2 copied to clipboard

Improve export-API so it cannot throw anymore

Open Lysander opened this issue 1 year ago • 1 comments
trafficstars

Problem

Until this PR, the export function was not safe to use:

class Exporter<T : Any>(initialize: Exporter<T>.() -> Unit) {
    lateinit var payload: T // this is bad and dangerous!

    fun export(payload: T): T {
        this.payload = payload
        return payload
    }

    init {
        initialize()
    }
}

As you can see there is the lateinit va payload: T, which gets only set, if export()-function gets calles in the initialize-Lambda.

This cannot be forced! And that leads to a brittle API :-(

Imagine some control structures like if, which prohibits the call if some value fails its condition. Then the outer export call will try to access the payload-field in order to return it. But it is still not initialized:

val something= false

export {
    if(something) {
        export("This wont get calles always!")
    }
}.also { /* an exception would be thrown before this block. Hard to find the precise code section on top! */ }

Solution

The export()-function. function now requires some new default: T parameter in order to enable the Scope-class to always return some valid value. The signature looks as follows:

fun <T : Any> export(default: T, scope: Exporter<T>.() -> Unit): T

As you can see this is unfortunately API breaking, but there was no other solution.

Migration Guide

Thus said, it should be quite simple to migrate to this new version. Just insert some senseful default value. Look at some example:

fun RenderContext.component(): HtmlTag<HTMLInputElement> = export(input {}) {
//                                                                ^^^^^^^^
//                                                                just create some empty Tag<HTMLInputElement>; 
//                                                                in this case it will never be called though.
    div(id = id) {
        export(
            input(id = inputId) {
                type("text")
            }
        )
    }
}

solves #878

Lysander avatar Aug 28 '24 16:08 Lysander