Use Builder pattern for class with multiple overlapping constructors

[espertusnu]

Is your suggestion for improvement related to a problem? Please describe.

The class AutoCompletionTextInputBinding has 3 different constructors with 3-5 parameters each, which does not follow best practices.

Describe the solution you'd like
I would like for one of my students to convert it to use the Builder pattern.

Additional context
@koppor agreed on Gitter that this would be a positive change.

I made the title more general in case we mutually agree to include other classes.

[espertusnu]

/assign-me

[github-actions]

👋 Hey @espertusnu, thank you for your interest in this issue! 🎉

We're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

For questions on JabRef functionality and the code base, you can consult the JabRef Guru or ask on our Gitter chat.

In case you encounter failing tests during development, please check our developer FAQs!

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

[espertusnu]

What would you think of adding these classes to the issue?

• LinkedFile
• BibDatabaseContext
• FXDialog

[Siedlerchr]

FXDialog is a bit problematic as it overwrites/extends the javafx class Alert and calls the super constructor.
BibdatabsaeContext would be better suited, I think.
Just be aware that BibDatabaseContext is used in many places and in tests. However, IntelliJ provides excellent features for helping with this.
https://www.jetbrains.com/help/idea/replace-constructor-with-builder.html

LinkedFiles would actually be an improvement, if you have a builder methods ilke withEmptyFile or withUrl that better state the intent. One has to really read the usages first, e.g. LInkedFiles has one special constructor:

jabref/jablib/src/main/java/org/jabref/model/entry/LinkedFile.java

Lines 64 to 72 in ca3821a

	/**
	* Constructor can also be used for non-valid paths. We need to parse them, because the GUI needs to render it.
	*/
	public LinkedFile(String description, String link, String fileType, String sourceUrl) {
	this.description.setValue(description);
	setLink(link);
	this.fileType.setValue(fileType);
	this.sourceURL.setValue(sourceUrl);
	}

[espertusnu]

Thank you. We won't touch FXDialog.

I was thinking of an internal Builder class for AutoCompletionTextInputBinding and BibDatabaseContext (and static factory methods for LinkedFile). We will definitely use IntelliJ's refactoring tools.

[koppor]

I think, jilt would be over-engineered. However, if sometone thinks, the code is more readable with jilt, I am open to use it.

There is also a builder especially for records - https://github.com/randgalt/record-builder. However, jilt also supports records. I did not evaluate, but found both projects interesting.

[Siedlerchr]

Please no third party library for such trivial things. I dare you.