PhoenicisOrg / phoenicis

Phoenicis PlayOnLinux and PlayOnMac 5 repository
https://phoenicis.org/
GNU Lesser General Public License v3.0
684 stars 73 forks source link

Adopt project lombok #1725

Open madoar opened 5 years ago

madoar commented 5 years ago

I suggest we use project lombok for Phoenicis to help us generate methods like getter, setter, hashcode and equals methods.

What do you think?

plata commented 5 years ago

Does it help a lot e.g. compared to getter/setter creation in IntelliJ?

qparis commented 5 years ago

This has already been discuess in the past: https://github.com/PhoenicisOrg/phoenicis/issues/262

The problem is mainly the compatibility with the dev tools

madoar commented 5 years ago

@qparis I've previously stumbled over #262, but I don't understand the issue discussed there. Project lombok can be used with maven.

@plata project lombok allows among others the creation of getter and setter methods via annotations. I.e. you can simply add a @Getter before a field in your class and during the compilation a getX() method is created.

qparis commented 5 years ago

With maven, yes. The problem was more about IDEs

madoar commented 5 years ago

This shouldn't be a huge problem:

More information about using project lombok with IDEs can be found here

qparis commented 5 years ago

Then, we can go ahead

qparis commented 5 years ago

But only if it really help to save time later. I think the codebase is pretty clean and we should more focus on features and performance improvement

plata commented 5 years ago

Agree to this. How much time will it really save us? Probably only very little. Also, it might be difficult for people who do not know it and want to contribute ("what is this strange annotation here?").

madoar commented 5 years ago

It is not really about saving time, but about saving lines in the source code. A lot of our classes, for example our JavaFX control classes or our DTO classes, consist of quite a lot setter and getter methods. If we can remove them we shorten our classes which makes it easier to find the relevant code inside said classes.

In addition lombok has quite a huge adoption as far as I know.

qparis commented 5 years ago

I agree that it is something that is very interesting, but at the end, is it really what our users are expecting for the next releases? (By user, I mean in the broad sense, i.e. developer included)

madoar commented 5 years ago

I don't want to forcefully adopt this now. It is just a thought I had in mind for quite a while. I opened this issue simply to discuss the issue and see what you think. If we want to go with it we can start by writing new classes with lombok. Later on, we can then change our existing classes to lombok.

qparis commented 5 years ago

Ok, then I think it's a good idea to keep this issue open.

plata commented 5 years ago

We could also write the data classes in Kotlin...

madoar commented 5 years ago

We could, but using two programming languages in one project would make our code base more complex. I would prefer a Java only solution.

If we really want to introduce a second programming language to Phoenicis, I would suggest Scala. It has almost all features of Kotlin and many more. A possible issue when using Scala is that not all the complex features/structures of Scala can be used from Java in an intuitive way (for example object functions and scala lists)

plata commented 5 years ago

I agree. Just stumbled over this...

opticyclic commented 5 years ago

IDEA doesn't have native (out of the box) support for lombok. It is provided by a plugin

If you don't regularly use lombok then you wont have the plugin and IDEA will think there are errors.

If you do switch then you need to add a section to the readme to tell people to add the plugin to their IDE before they can start developing.

IMO it doesn't gain you anything at all.

It is not really about saving time, but about saving lines in the source code.

Its not like you are saving money by having fewer lines in the source code. Its not duplicate code that needs refactoring. Its just a case of "ooh this class looks nicer because I only have properties in it and no methods".

A lot of our classes, for example our JavaFX control classes or our DTO classes, consist of quite a lot setter and getter methods. If we can remove them we shorten our classes which makes it easier to find the relevant code inside said classes.

The relevant code in this case is just the class properties, which is at the top of the class in both cases so you haven't gained anything.

Everything that lombok provides is automatically generated by IDEA anyway so you don't gain anything there either. Its not like people are manually typing the getters and setters.

madoar commented 5 years ago

Its not like you are saving money by having fewer lines in the source code. Its not duplicate code that needs refactoring. Its just a case of "ooh this class looks nicer because I only have properties in it and no methods".

I don't agree. Having less lines of code and possible shorter lines of code allows for a better overview of the code. This normally makes it also much easier to understand what a class contains and what it does.

The relevant code in this case is just the class properties, which is at the top of the class in both cases so you haven't gained anything.

Everything that lombok provides is automatically generated by IDEA anyway so you don't gain anything there either. Its not like people are manually typing the getters and setters.

I agree in the case when your class only contains getter and setter methods and no other types of methods. If your class contains also other methods getter and setter methods are kind of annoying, especially in larger numbers, because they make it hard to find the "actual" methods inside the code.

plata commented 5 years ago

@madoar does lombok work properly with Gitpod?