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

Idiomatic setters/getters #13

Open
MattSturgeon opened this issue Apr 4, 2024 · 1 comment
Open

Idiomatic setters/getters #13

MattSturgeon opened this issue Apr 4, 2024 · 1 comment
Labels
enhancement New feature or request future update To be added to a future update

Comments

@MattSturgeon
Copy link
Contributor

MattSturgeon commented Apr 4, 2024

Kotlin and Groovy both have some built-in coersion of getters and setters into plain object properties.

In Groovy, a getters and setters form what we call a "property", and offers a shortcut notation for accessing and setting such properties.
~Groovy Style guide: Getters and Setters

Methods that follow the Java conventions for getters and setters (no-argument methods with names starting with get and single-argument methods with names starting with set) are represented as properties in Kotlin
~Calling Java from Kotlin

I believe kotlin's implementation may be incompatible with lombok's default is prefix for boolean methods.

If this is the only issue, it may be fixed by setting lombok.getter.noIsPrefix = true in a lombok.config file.

You can create lombok.config files in any directory and put configuration directives in it. These apply to all source files in this directory and all child directories.
~Configure lombok features


This would need some experimentation, I've noticed when playing around in kotlin codebases that some classes get synthetic properties while others don't.

Examples

E.g. GHRepository.getFullName() works:

image

So does GitHub:

image

While some stuff on File works:

image

And other stuff just doesn't:

image

Maybe this is because setReadOnly() isn't actually setting a field, instead it's probably doing filesystem IO.


I believe this would be best as a breaking change, since it'd be a pain to introduce idiomatic setters while still maintaining the existing API. That said, it should be possible and may not be too bad if you think it's worth it...

@hypherionmc
Copy link
Member

Wow, this is very insightful.

I agree that this is definitely worth checking out and experimenting with, but might be best kept for a Major release, instead of minor (so maybe 3.0.0).

I have been wanting to expand to other upload platforms, such as [Hangar](https://hangar.papermc.io] from papermc, and our own NightBloom snapshots platform.

I also want to redo the entire CurseForge side, since their API has gotten some new stuff that isn't properly supported, and just smacked in because it was needed, and then the modrinth ID caching and all that good stuff.

@hypherionmc hypherionmc added enhancement New feature or request future update To be added to a future update labels Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request future update To be added to a future update
Projects
Status: No status
Development

No branches or pull requests

2 participants