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

Allow sets to have an empty default value #232

Merged
merged 1 commit into from
Aug 4, 2014
Merged

Allow sets to have an empty default value #232

merged 1 commit into from
Aug 4, 2014

Conversation

bitfrost
Copy link
Contributor

@bitfrost bitfrost commented Aug 1, 2014

No description provided.

@joelittlejohn
Copy link
Owner

Hi @bitfrost, thanks for submitting this, could you help me understand why this PR is needed? An empty set will be created already if you omit the "default" rule, and (as far as I can tell) you'll already get a default empty set if your schema has default: [].

Are you combining this with initializeCollections=false?

One other small thing: if this change is needed then we should apply it to both getDefaultSet and getDefaultList.

@bitfrost
Copy link
Contributor Author

bitfrost commented Aug 4, 2014

Hi @joelittlejohn, I am sorry I did not see your reply until this morning,
I want to specify an empty array explicitly for javascript usage of the schema, but the following valid schema results in a compile error for the generated java code.

This change as far as I can tell was already been applied to getDefaultList to close similar issue.
I am not using initializeCollections=false. getDefaultList/getDefaultSet are very similar, i considered collapsing them to one function instead of making this 1 line change, but got lazy :)

Example:

"widgets" : {
      "description" : "An array of widgets.",
      "type" : "array",
      "minItems" : 0,
      "items" : {
          "$ref" : "widget.json"
      },
      "default" : [],
      "uniqueItems" : true
    }
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project dashboard-service: Compilation failure
[ERROR] /Users/kobrien/dev/dashboard-service/target/generated-sources/com/cvent/board/core/Dashboard.java:[158,35] no suitable constructor found for LinkedHashSet(java.util.List<java.lang.Object>)
[ERROR] constructor java.util.LinkedHashSet.LinkedHashSet(java.util.Collection<? extends com.cvent.board.core.Widget>) is not applicable
[ERROR] (actual argument java.util.List<java.lang.Object> cannot be converted to java.util.Collection<? extends com.cvent.board.core.Widget> by method invocation conversion)
[ERROR] constructor java.util.LinkedHashSet.LinkedHashSet() is not applicable
[ERROR] (actual and formal argument lists differ in length)
[ERROR] constructor java.util.LinkedHashSet.LinkedHashSet(int) is not applicable
[ERROR] (actual argument java.util.List<java.lang.Object> cannot be converted to int by method invocation conversion)
[ERROR] constructor java.util.LinkedHashSet.LinkedHashSet(int,float) is not applicable
[ERROR] (actual and formal argument lists differ in length)

Offending generated code:

  private Set<Widget> widgets = new LinkedHashSet<Widget>(Arrays.asList());

@joelittlejohn
Copy link
Owner

Thanks for your reply, I see exactly what you mean now.

joelittlejohn added a commit that referenced this pull request Aug 4, 2014
allow sets to have an [ ] (empty) default value
@joelittlejohn joelittlejohn merged commit ae0bd0e into joelittlejohn:master Aug 4, 2014
@joelittlejohn joelittlejohn added this to the 0.4.5 milestone Aug 4, 2014
@joelittlejohn joelittlejohn changed the title allow sets to have an [ ] (empty) default value Allow sets to have an [ ] (empty) default value Aug 4, 2014
@joelittlejohn joelittlejohn mentioned this pull request Aug 11, 2014
@joelittlejohn joelittlejohn changed the title Allow sets to have an [ ] (empty) default value Allow sets to have an empty default value Oct 31, 2014
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.

2 participants