Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java APIs: Add a Builder interface implemented by all Builders #1652

Closed
1 of 2 tasks
GrahamLea opened this issue May 8, 2020 · 0 comments · Fixed by #1654
Closed
1 of 2 tasks

Java APIs: Add a Builder interface implemented by all Builders #1652

GrahamLea opened this issue May 8, 2020 · 0 comments · Fixed by #1654
Assignees
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p2

Comments

@GrahamLea
Copy link

In Java, each Builder class should implement a generic Builder interface.

Use Case

We can use extension methods in Kotlin to make it less verbose to build props, like this:

        fun stackProps(): StackProps =
            StackProps.Builder().props {
                env(Environment.Builder().props {
                    account(accountToUse)
                    region(regionToUse)
                })
            }

which works by using this extension method:

        fun <B : Any, T> B.props(init: B.() -> Unit): T {
            init(this)
            val buildFunction = this::class.memberFunctions.single { it.name == "build" }
            return buildFunction.call(this) as T
        }

Unfortunately, the extension method has to use unsafe reflection and casting due to there being no relationship in the type system between Builders and the types they build.

A simple Builder interface would help make helper methods like these simpler and type-safe, like this:

        fun <B: Builder<T>, T> B.props(init: B.() -> Unit): T {
            init(this)
            return this.build()
        }

Proposed Solution

The Java API should include this interface:

package software.amazon.awscdk.core;

@FunctionalInterface
public interface Builder<T> {
    T build();
}

and each Builder class should implement it:

public interface StackProps extends software.amazon.jsii.JsiiSerializable {
    ....
    public static final class Builder implements software.amazon.awscdk.core.Builder<StackProps> {
        ....

Other

This should be backwards-compatible for almost all cases. In theory, it simply adds an interface for functions that are already present. In the wild, if some code relies on the fact that Builder classes don't implement an interface, that code may break.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@RomainMuller RomainMuller transferred this issue from aws/aws-cdk May 12, 2020
@RomainMuller RomainMuller added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p2 labels May 12, 2020
RomainMuller added a commit that referenced this issue May 12, 2020
The common super-interface for all `Builder` classes creates a nice and
clean extension point that can be leveraged to improve the experience of
developers using Kotlin extensions, or doing fancy AOP things.

The new interface does not change the current `Builder` layout, merely
adding the new super-interface to pre-declare an already-existing method.

Fixes #1652
@mergify mergify bot closed this as completed in #1654 May 13, 2020
mergify bot pushed a commit that referenced this issue May 13, 2020
…1654)

The common super-interface for all `Builder` classes creates a nice and
clean extension point that can be leveraged to improve the experience of
developers using Kotlin extensions, or doing fancy AOP things. For example,
this enables introduction of Kotlin extensions to improve/simplify the syntax
needed to initialize types that provide a `Builder` further from what the
Java implementation natively allows (see the related issue for more info).

The new interface does not change the current `Builder` layout, merely
adding the new super-interface to pre-declare an already-existing method.

Fixes #1652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants