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: Improve use of Builder pattern #488

Closed
fulghum opened this issue May 3, 2019 · 1 comment · Fixed by #895
Closed

java: Improve use of Builder pattern #488

fulghum opened this issue May 3, 2019 · 1 comment · Fixed by #895
Assignees
Labels
effort/large Large work item – several weeks of effort language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p0

Comments

@fulghum
Copy link

fulghum commented May 3, 2019

UX Research Feedback – Customers have commented that the use of Builder objects for CDK function prop objects does not feel idiomatic. They find passing in a builder as the third param for construct construction inconsistent with other Java APIs and the builder pattern.

Customers expected to see the Builder pattern either used entirely for constructing constructs, or not at all in the constructor.

@fulghum fulghum added the language/java Related to Java bindings label May 3, 2019
@RomainMuller RomainMuller added this to the Java Support milestone Jul 30, 2019
@fulghum fulghum added effort/large Large work item – several weeks of effort p0 labels Aug 13, 2019
@RomainMuller RomainMuller added the module/pacmak Issues affecting the `jsii-pacmak` module label Oct 4, 2019
@RomainMuller
Copy link
Contributor

RomainMuller commented Oct 17, 2019

The design we are going for is:

Table configTable = TableBuilder.builder(this, "configTable")
        .tableName(configTableInfo.getDynamoTableName())
        .billingMode(BillingMode.PAY_PER_REQUEST)
        .serverSideEncryption(true)
        .partitionKey(Attribute.builder()
                .name(tableMeta.getPartitionKeyField().getAttributeName())
                .type(pkType)
                .build())
        .build();

RomainMuller added a commit that referenced this issue Oct 18, 2019
Provide a nested `Builder` class with a static `create` factory for all
classes that adhere to the following constraints:
- The class is not `abstract`
- The class has a visible `initializer` (aka `constructor`)
- The constructor has exactly one `struct` (aka `datatype`) parameter
- The constructor is not variadic

Such builders will request the *positional* parameters to be passed
directly to the `Builder#create` method and can (at least currently) not
be subsequently modified. All parameters from the `struct` are then set
direcly on the `Builder` instance.

Fixes #488
@mergify mergify bot closed this as completed in #895 Oct 29, 2019
mergify bot pushed a commit that referenced this issue Oct 29, 2019
* feat(java): offer Builders for certain Java classes

Provide a nested `Builder` class with a static `create` factory for all
classes that adhere to the following constraints:
- The class is not `abstract`
- The class has a visible `initializer` (aka `constructor`)
- The constructor has exactly one `struct` (aka `datatype`) parameter
- The constructor is not variadic

Such builders will request the *positional* parameters to be passed
directly to the `Builder#create` method and can (at least currently) not
be subsequently modified. All parameters from the `struct` are then set
direcly on the `Builder` instance.

Fixes #488

* improvements based on PR feedback from @bmaizels

* add factory overrides, better documentation coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/large Large work item – several weeks of effort language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants