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

Add Info.beanify() to add JavaBeans-style prefixes for getter and setters of member variables generated by Parser #495

Merged
merged 2 commits into from
Jun 16, 2021

Conversation

equeim
Copy link
Contributor

@equeim equeim commented Jun 5, 2021

Kotlin's feature of automatic conversion of Java getters/setters to properties requires them to have JavaBeans-style prefixes (unless class is a record). Parser doesn't add them which makes classes generated by it to be awkward to use in Kotlin.

@saudet
Copy link
Member

saudet commented Jun 5, 2021

Let's name that Info.beanify? And instead of annotating with @Name, let's add the logic for that in Generator instead.

@equeim
Copy link
Contributor Author

equeim commented Jun 6, 2021

And instead of annotating with @name, let's add the logic for that in Generator instead.

How will Generator distinguish between member that was beanified and member which C++ name actually starts wit "get"? If we have both getter and setter, then we can make a guess because parts of method names after get/set prefixes are the same and therefore they are getter and setter and extract actual member name by stripping prefix and lowercasing first character. However, we can't do that if we have only getter.

Also, if C++ member name is capitalized, then its getter/setter method name will be the same as for member that start with lowercase character.

@saudet
Copy link
Member

saudet commented Jun 8, 2021

And instead of annotating with @name, let's add the logic for that in Generator instead.

How will Generator distinguish between member that was beanified and member which C++ name actually starts wit "get"? If we have both getter and setter, then we can make a guess because parts of method names after get/set prefixes are the same and therefore they are getter and setter and extract actual member name by stripping prefix and lowercasing first character. However, we can't do that if we have only getter.

Also, if C++ member name is capitalized, then its getter/setter method name will be the same as for member that start with lowercase character.

Conflicts can occur, yes, it happens with the current scheme as well, that's not a problem. That's what the @Function, @MemberGetter, @Name, etc annotations are for. We can use them when needed, but it's nice to support common cases without tons of annotations everywhere. I haven't seen a lot of C++ APIs that use JavaBean-like namings, so I don't foresee any problems.

@equeim equeim changed the title Add Info.addMemberPrefixes() to add JavaBeans-style prefixes for getter and setters of member variables generated by Parser Add Info.beanify() to add JavaBeans-style prefixes for getter and setters of member variables generated by Parser Jun 8, 2021
@saudet
Copy link
Member

saudet commented Jun 11, 2021

Hum, I've been testing this against the presets, and the get/set pattern is used in C++ APIs more frequently than I thought.
Let's revert that to the Generator-less solution, unless you have a better idea?

/cc @HGuillemet

… setters of member variables generated by `Parser`
@equeim
Copy link
Contributor Author

equeim commented Jun 11, 2021

Not really. I realize that Parser-only solution is not very convenient when writing Java classes by hand instead of using Parser, but it still the most reliable one and (IMO) least confusing. I suspect that it is possible to make Generator detection work better, but it will add event more complexity to it. So I reverted back to previous solution (renaming to beanify).
BTW, one of CI jobs seems to be stuck.

@saudet saudet merged commit 9add16e into bytedeco:master Jun 16, 2021
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