swagger-api
Issue 7638
PR checklist
- [x] Read the contribution guidelines.
- [x] Ran the shell script under
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\. - [x] Filed the PR against the correct branch:
master. - [x] Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
Description of the PR
As described in Issue 7638 there is now the option to define the getter-prefix for Booleans. For those who favor isValid (or maybe hasValue) instead of getValid. Default is get.
Option example: --boolean-getter-prefix is or --boolean-getter-prefix has
@bbdouglas, @JFCote, @sreeshas, @jfiala, @lukoyanov, @cbornet, @jeff9finger: Please verify
I have have not changed the generation in AbstractCppCodegen which is still by default is. Same is true for SymfonyServerCodegen (PHP). Maybe it would be good to clean up these exceptional cases or keep it like i did in AbstractCppCodegen (C++).
Last remark: Note that it would even be better to have is-prefix for primitive boolean and get-prefix for Boolean type.
I assume that this new option will be available from the maven plugin as well.
If so, LGTM
This PR resolves https://github.com/swagger-api/swagger-codegen/issues/7638
I don’t think this should be a global option. Most generators don’t care about it
I even don’t think this should be an option at all. Boolean wrapper should be called with get, not is. Always.
@cbornet @dkellenb To be honest, I think this is the same kind of debate that we could have for indentation style, tabs vs space, etc... I think the more we provide ways for the user to customize the output of swagger-codegen, the better it is.
I agree with you that most generators don't care because they don't have the concept of getter/setter (think of typescript) or it's another concept completely (C# with properties) Maybe this should be implemented on a selected few generators (java mostly I think)? It could be in the AbstractJavaCodegen file?
As for always using "get" like @cbornet said, many people would disagree with him. Using "is" in the getter name help to see if the name of the variable is ok: https://stackoverflow.com/questions/5322648/for-a-boolean-field-what-is-the-naming-convention-for-its-getter-setter
I have projects who use both.
So maybe we should move this to the AbstractJavaCodegen as an option instead of a general option?
I agree with @JFCote that it is better to have the getter naming customizable.
It is even more important since some property mappers do not map Boolean isMyVariable() but only boolean isMyVariable() or Boolean getMyVariable(), see e.g. in the Spring BeanWrapper: https://jira.spring.io/browse/SPR-9482
However, I would suggest to leave it in the default codegen, and not move it to the abstract java codegen, since the property-mapping issue could possibly also exist in other languages.
I think the more we provide ways for the user to customize the output of swagger-codegen, the better it is.
Sorry but I disagree. The more options there are, the harder it is to maintain, refactor, evolve, etc... I think we already have too many options on the Java gen which has become very messy (we should do something about that at some point btw).
As for always using "get" like @cbornet said, many people would disagree with him. Using "is" in the getter name help to see if the name of the variable is ok: https://stackoverflow.com/questions/5322648/for-a-boolean-field-what-is-the-naming-convention-for-its-getter-setter
This SOF link is about the boolean primitive, not the Boolean wrapper type. While it's OK to use the is getter for boolean, it's not allowed for the Boolean wrapper in the JavaBeans spec as @macjohnny pointed out.
