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

Adds support for builder API generation in Java #154

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

desaikd
Copy link
Contributor

@desaikd desaikd commented Oct 18, 2024

Issue #97

Description of changes:

This PR adds support for builder API generation in Java.

Generated code in Java:

Generated code adds builder API support which can be found here.

List of changes:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* Adds nested builder class
* `build` mehtod verifies required/optional fields
* `readFrom` usess new builder to create an instance of the class
Copy link
Contributor

@popematt popematt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have builders, can you also make the setters private, please?

src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
@@ -17,7 +17,7 @@ import java.io.IOException;
class {{ model.name }} {
private java.util.ArrayList<{{ sequence_info["element_type"] | fully_qualified_type_name }}> value;

public {{ model.name }}() {}
private {{ model.name }}() {}

public java.util.ArrayList<{{ sequence_info["element_type"] | fully_qualified_type_name }}> getValue() {
Copy link
Contributor

@popematt popematt Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I missed this earlier. When working with Java collections, you should use the widest acceptable interface for the use case in the API (so List in this case) and use the specific implementation only when you're actually constructing a new instance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, It's fine if you want to follow up on this in another PR.

src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
src/bin/ion/commands/generate/templates/java/class.templ Outdated Show resolved Hide resolved
@desaikd desaikd merged commit 61b34e3 into amazon-ion:new-model-changes Oct 21, 2024
3 checks passed
desaikd added a commit to desaikd/ion-cli that referenced this pull request Nov 1, 2024
desaikd added a commit that referenced this pull request Nov 1, 2024
* Adds new model changes for `Structure` (#138)
* Adds new model changes for scalar (#143)
* Adds changes for sequence data type (#144)
* Adds tests for new model changes (#146)
* Adds new model changes for Rust code generation (#147)
* Adds changes for optional/required fields in Java code generation (#148)
* Modifies generated setter API to take nested types instead of nested properties (#153)
* Adds support for builder API generation in Java (#154)
* Adds support for Java enum generation (#158)
* Adds namespace fix for nested sequence and adds support for nested types in Sequence and Scalar ADT (#163)
* Adds changes for nested type naming (#166)
* Adds support for imports in code generation (#169)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants