Miscellaneous refactoring #13178

subhramit · 2025-05-27T23:18:45Z

use strict types throughout the codebase
convert multi-ifs to switch
remove some unused variables
remove some unused exceptions
fix indents
use toList and List.of at some places where list is not modified
use javascript let in PreviewViewer script

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[/] Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
[/] Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…les and exceptions, convert ifs to switch, etc. Signed-off-by: subhramit <[email protected]>

trag-bot · 2025-05-27T23:21:41Z

@trag-bot didn't find any issues in the code! ✅✨

jabgui/src/main/java/org/jabref/gui/dialogs/BackupUIManager.java

jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java

trag-bot · 2025-05-27T23:29:27Z

jabgui/src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java

        EasyBind.subscribe(viewModel.selectedEntryTypeProperty(), type -> {
            if (type != null) {
-                var items = type.fields();
+                ObservableList<FieldViewModel> items = type.fields();


The patch replaces 'var' with 'ObservableList', which is correct for type safety, but it should also consider using modern Java data structures as per best practices.

trag-bot · 2025-05-27T23:32:42Z

jabgui/src/main/java/org/jabref/gui/util/FileFilterConverter.java


    public static FileFilter toFileFilter(List<String> extensions) {
-        var filter = toDirFilter(extensions);
+        Filter<Path> filter = toDirFilter(extensions);


The explicit type declaration 'Filter' is preferred over 'var' for clarity and type safety, aligning with modern Java best practices.

trag-bot · 2025-05-27T23:34:44Z

jabgui/src/test/java/org/jabref/gui/entryeditor/SciteTabViewModelTest.java

        jsonObject.put("doi", "test_doi");

-        var dto = SciteTallyModel.fromJSONObject(jsonObject);
+        SciteTallyModel dto = SciteTallyModel.fromJSONObject(jsonObject);


The variable name 'dto' is not descriptive enough. It should include meaning or intention to improve code readability and maintainability.

trag-bot · 2025-05-27T23:34:45Z

jabgui/src/test/java/org/jabref/gui/entryeditor/SciteTabViewModelTest.java

    @Test
    void fetchTallies() throws FetcherException {
-        var viewModel = new SciteTabViewModel(preferences, taskExecutor);
+        SciteTabViewModel viewModel = new SciteTabViewModel(preferences, taskExecutor);


The variable name 'viewModel' is not descriptive enough. It should include meaning or intention to improve code readability and maintainability.

trag-bot · 2025-05-27T23:37:00Z

...c/test/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesViewModelTest.java


        ConstantsPropertiesViewModel model = new ConstantsPropertiesViewModel(context, service, externalApplicationsPreferences);
-        var stringsList = model.stringsListProperty();
+        ListProperty<ConstantsItemModel> stringsList = model.stringsListProperty();


The patch replaces var with ListProperty, but the special instructions suggest using modern Java data structures, which might include using more specific types or methods like List.of().

trag-bot · 2025-05-27T23:37:34Z

jabgui/src/test/java/org/jabref/gui/mergeentries/FieldRowViewModelTest.java


    @BeforeEach
-    void setup() throws ParseException {
+    void setup() {


The method signature should use 'throws Exception' instead of removing the exception handling to align with best practices for test methods.

trag-bot · 2025-05-27T23:38:04Z

jabgui/src/test/java/org/jabref/gui/search/GlobalSearchBarTest.java


        // The focus is on searchField node, as we click on the search box
-        var searchFieldRoboto = robot.clickOn(searchField);
+        FxRobotInterface searchFieldRoboto = robot.clickOn(searchField);


The variable name 'searchFieldRoboto' does not follow camel-case naming convention. It should be 'searchFieldRobot' to maintain consistency with naming standards.

subhramit · 2025-05-28T00:07:49Z

Why are the windows unit tests taking so long?

Update: finished in 33 min

koppor · 2025-05-28T07:11:05Z

Why are the windows unit tests taking so long?

IDK. Maybe a general GitHub actions issue?

koppor · 2025-05-28T07:15:49Z

jablib/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

+                    File[] subDirs = actualDirectory.toFile().listFiles();
+                    if (subDirs != null) {
+                        String restOfFileString = StringUtil.join(fileParts, "/", index + 1, fileParts.length);
+                        for (File subDir : subDirs) {
+                            if (subDir.isDirectory()) {
+                                resultFiles.addAll(findFile(entry, subDir.toPath(), restOfFileString, extensionRegExp));
+                            }


In a follow-up, this could be modernized, too.

Rename to currentDirectory (actual is IMHO a false friend)

Try to use walkFileTree -> https://docs.oracle.com/javase/tutorial/essential/io/walk.html

Can't be done be Java newcomers as reviewing of the resulting code is too hard ^^

Siedlerchr · 2025-05-28T07:42:24Z

jablib/src/main/java/org/jabref/generators/LtwaListMvGenerator.java

        LOGGER.info("Downloading LTWA file from {}.", LtwaListMvGenerator.LTWA_URL);
-        var in = new URI(LTWA_URL).toURL().openStream();
-        var path = Files.writeString(
+        InputStream inputStream = new URI(LTWA_URL).toURL().openStream();


Follow up for future: Use try with resources statement

subhramit added 2 commits May 28, 2025 04:44

Refactor: use strict types, toList, List.of, remove unused variab…

e14ed15

…les and exceptions, convert ifs to switch, etc. Signed-off-by: subhramit <[email protected]>

Merge branch 'main' into strict-types

e4c098e