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

Copy Constructor does not utilize isSetXXX methods when they are generated. #8

Closed
fericit-bostan opened this issue Sep 14, 2015 · 7 comments

Comments

@fericit-bostan
Copy link

I am utilizing -immutable, -imm-builder and -imm-cc configuration flags. The Builder's copy constructor is generated as are all of the isSetXXX methods. The problem is that the copy constructor is invoking all of the getXXX methods of the supplied object rather than performing a copy of the fields. The side effect is that if the supplied Column's field has not been set and has a default value specified for a field then the default value is copied over rather than leaving the field's value as null.

One of two actions should occur:
1 - the copy operation should copy the fields value and not invoke the getXXX methods.
2 - if the isSetXXX methods are generated then the copy operation should be as follows:
if(o.isSetXXX()) {
this.XXX = o.getXXX();
}

In my opinion, option 1 is the easier solution to implement and would still honor the contract of the isSetXXX methods.

By the way, great job on this implementation. I find it very clean and I appreciate the clean, generated code very much. A job well done!!!

@sabomichal
Copy link
Owner

Thanks for filing the issue, you are right, it will be fixed.

@fericit-bostan
Copy link
Author

When do you think that you will release the next version of the plugin? I'd really like to pickup the changes.

Thanks for giving the issue attention.

@sabomichal
Copy link
Owner

Since you were giving this repo a star, I'll try to do it ASAP. :)

@sabomichal
Copy link
Owner

Check if current HEAD is working for you, I will then do the release. Nevertheless isSetXXX methods are not generated by default (I think you are leveraging the jaxb:bindings), so they were no use. I used the first proposed solution, however just for the base class. Assignment of the superclass fields can be done just by calling the getters as in the original solution.

@sabomichal
Copy link
Owner

And as I see the commit was not interpreted correctly by git, so it looks like the whole file was changed. :-/

@fericit-bostan
Copy link
Author

I just cloned the repo and installed the build. I ran my project and the generated code (copy constructor) looks good.

I didn't think the isSetXXX was a good solution to the issue, and I had forgotten that I had asked jaxb to generate those methods, so you are correct in that it would not have worked. This is the better solution to the problem.

Thank you for providing a fix for this so quickly. I'll look for the release.

@sabomichal
Copy link
Owner

released under version 1.3.1

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

No branches or pull requests

2 participants