Added the feature of IcoRanking #13512

asifebrahim · 2025-07-08T20:09:31Z

This is the PR request for issue no #13476

Steps to test

In the General field at the very bottom there sthe icoranking lookup fiield there one can search using a acronym and get the details about the conference.

NOTE

-The Abbreviation tests were failing before i added any code so there were a total of 76 failed test case sand after my addition it is still 76 and all of them are NullPointerException and I didn't fix that

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.

trag-bot · 2025-07-08T20:11:22Z

jabgui/src/main/java/module-info.java

@@ -11,6 +11,7 @@
    requires java.sql.rowset;

    // region JavaFX
+    requires javafx.swing;


The module requires both Swing and JavaFX dependencies, which violates the architectural decision to use only JavaFX for UI components. Swing dependencies should be removed.

remove the dep here

trag-bot · 2025-07-08T20:13:37Z

jabgui/src/test/java/org/jabref/gui/fieldeditors/FieldEditorsTest.java

+
+    @Test
+    void testEditorCreationDoesNotCrash() {
+        assertNotNull(FieldEditors.class);


According to modern testing practices, should assert specific contents/behavior rather than just checking for non-null conditions.

Remove this test - it is always true

trag-bot · 2025-07-08T20:14:33Z

jablib/src/main/java/org/jabref/logic/icore/ConferenceRankingEntry.java

+    public final String title;
+    public final String acronym;
+    public final String source;
+    public final String rank;
+    public final String note;
+    public final String dblp;
+    public final String primaryFor;
+    public final String comments;
+    public final String averageRating;


Class fields are declared as public, violating encapsulation principles. They should be private with getter methods to maintain proper encapsulation and control over data access.

Convert the class to record

trag-bot · 2025-07-08T20:16:59Z

jablib/src/test/java/org/jabref/logic/icore/ICoreRankingRepositoryTest.java

+        assertTrue(repo.getRankingFor("NON_EXISTENT").isEmpty());
+    }
+    @Test
+    public void test1(){


Test method name is not descriptive enough. It should clearly indicate what is being tested, such as 'testFuzzyMatchForInternationalConferenceOnSoftwareEngineering'.

trag-bot · 2025-07-08T20:17:34Z

jablib/src/test/java/org/jabref/logic/util/ConferenceUtilsTest.java

+        assertEquals(Optional.empty(), ConferenceUtil.extractAcronym(title));
+    }
+    @Test
+    void test1(){


Test method name is not descriptive enough and doesn't indicate what is being tested. Should describe the test scenario like 'testExtractAcronymFromConferenceWithParentheses'.

trag-bot · 2025-07-08T20:17:36Z

jablib/src/test/java/org/jabref/logic/util/ConferenceUtilsTest.java

+        assertEquals(Optional.of("ICSE"),ConferenceUtil.extractAcronym(title));
+    }
+    @Test
+    void test2(){


Test method name is not descriptive enough and doesn't indicate what is being tested. Should describe the test scenario like 'testExtractAcronymFromConferenceWithoutParentheses'.

koppor · 2025-07-09T01:36:32Z

jabgui/src/test/java/org/jabref/gui/fieldeditors/FieldEditorsTest.java

+
+    @Test
+    void testEditorCreationDoesNotCrash() {
+        assertNotNull(FieldEditors.class);


Remove this test - it is always true

koppor · 2025-07-09T01:36:58Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/ICoreRankingEditor.java

+    public void bindToEntry(BibEntry entry) {
+        this.currentEntry = entry;
+
+//        System.out.println("ENTRY booktitle = " + entry.getField(StandardField.BOOKTITLE).orElse("none"));


Use LOGGER.debug - see https://devdocs.jabref.org/code-howtos/logging.html

koppor · 2025-07-09T01:37:22Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/ICoreRankingEditor.java

+    Optional<String> acronym = ConferenceUtil.extractAcronym(rawInput); // Extracting the acronym from our input field
+    Optional<ConferenceRankingEntry> result = acronym.flatMap(repo::getFullEntry)
+            .or(() -> repo.getFullEntry(rawInput));  // Finding if any matching entry present in csv file
+// If present then print the info


Reformat the comments -- starting at column 1 is not ok

koppor · 2025-07-09T01:37:59Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/ICoreRankingEditor.java

+        this.setSpacing(10);
+    }
+
+    // Deprecated method it only shows rank by puttin the acronym in the text field


Not sure what this code does.

I think, this should be removed -- it is part of some logic in all cases.

koppor · 2025-07-09T01:39:03Z

jablib/src/main/java/org/jabref/logic/icore/ConferenceRankingEntry.java

+            Primary FoR: %s
+            Comments: %s
+            Average Rating: %s
+            """, title, acronym, source, rank, note, dblp, primaryFor, comments, averageRating);


It is OK to have an additional way of showing the contents...

However, in future, use IntelliJ's feature to create a toString method

koppor · 2025-07-09T01:49:18Z

jablib/src/main/java/org/jabref/logic/icore/ICoreRankingRepository.java

+
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+            reader.lines().skip(1).forEach(line -> {
+                String[] parts = line.split(",(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)", -1);


No.

There are libraries for this. In JabRef, we use https://commons.apache.org/proper/commons-csv/

koppor · 2025-07-09T01:50:23Z

jablib/src/main/java/org/jabref/logic/icore/ICoreRankingRepository.java

+        LOGGER.info("Fuzzy match fallback triggered for: " + key);
+        return nameToRank.entrySet().stream()
+                .filter(e -> similarity.editDistanceIgnoreCase(e.getKey(), key) < 0.01)
+                .peek(e -> LOGGER.info("Fuzzy match candidate: " + e.getKey()))


Wrong logger format - use {} as place holder. See other code in JabRef.

(also applies for all other logger statements)

jablib/src/main/resources/ICORE.csv

koppor · 2025-07-09T01:52:14Z

jablib/src/main/java/org/jabref/logic/icore/ConferenceRankingEntry.java

+    public final String title;
+    public final String acronym;
+    public final String source;
+    public final String rank;
+    public final String note;
+    public final String dblp;
+    public final String primaryFor;
+    public final String comments;
+    public final String averageRating;


Convert the class to record

koppor · 2025-07-09T01:53:54Z

-The Abbreviation tests were failing before i

Maybe, you did not check out JabRef correctly. Execute git submodule update on the command line.

koppor · 2025-07-09T06:46:04Z

Regarding the output information

1: Should be link -- See other places in JabRef how to open a browser
2: Remove - this information is not useful

koppor · 2025-07-09T06:49:26Z

@asifebrahim You screenshort shows Icoranking as field name. It is ICORE, which should be rendered as Icoreranking in JabRef. You missed two letters. Also in your title in this PR, you missed two letters. Please engineer properly 😅.

koppor · 2025-07-09T06:50:09Z

Even though the larger button is more accessible - the current JabRef style is to use icons. Please use the same button style as in the DOI

koppor · 2025-07-09T06:59:38Z

You also need to do a preference migration - icoreranking needs to be added to general

Add a new method to org.jabref.migrations.PreferencesMigrations
Method is similar to org.jabref.migrations.PreferencesMigrations#removeCommentsFromCustomEditorTabs
Method has to check if the current content is General:doi;crossref;keywords;eprint;url;file;groups;owner;timestamp;printed;priority;qualityassured;ranking;readstatus;relevance. If yes, change it to doi;icoreranking;...

Also add the field directly after DOI to org.jabref.model.entry.field.FieldFactory#getDefaultGeneralFields.

trag-bot · 2025-07-09T16:52:31Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/ICoreRankingEditor.java

+    Optional<String> finalBookTitle = bookTitle;
+    String rawInput = icoreField.orElseGet(() -> finalBookTitle.orElse("Unknown"));
+
+    Optional<String> acronym = ConferenceUtil.extractAcronym(rawInput); // Extracting the acronym from our input field


Comment is trivial and can be derived from the code itself. The comment doesn't add any new information about why the acronym extraction is needed.

trag-bot · 2025-07-09T16:52:33Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/ICoreRankingEditor.java

+    public ICoreRankingEditor(Field field) {
+        this.field = field;
+        this.textField = new TextField();
+        this.repo = new ICoreRankingRepository(); // Load once


Comment 'Load once' is trivial and doesn't provide additional information. The single initialization in the constructor already implies this.

trag-bot · 2025-07-09T16:56:46Z

jabgui/src/main/java/org/jabref/migrations/PreferencesMigrations.java

 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

 public class PreferencesMigrations {

    private static final Logger LOGGER = LoggerFactory.getLogger(PreferencesMigrations.class);
+    private JabRefGuiPreferences preferences;


Field should be final to prevent modification after initialization, following immutability best practices and thread-safety principles of modern Java.

trag-bot · 2025-07-09T16:56:49Z

jabgui/src/main/java/org/jabref/migrations/PreferencesMigrations.java

-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;


Using wildcard imports instead of specific imports reduces code readability and can lead to naming conflicts. Each import should be explicitly declared.

trag-bot · 2025-07-09T16:58:26Z

jablib/src/main/java/org/jabref/logic/icore/ConferenceRankingEntry.java

+        String note,
+        String dblp,
+        String primaryFor,
+        String averageRating


Using String for numerical data prevents proper validation and comparison. Should use Double or BigDecimal to ensure proper numerical handling and prevent invalid data entry.

trag-bot · 2025-07-09T16:59:08Z

jablib/src/main/java/org/jabref/logic/icore/ICoreRankingRepository.java

+    final Map<String, String> acronymToRank = new HashMap<>();
+    private Map<String, String> nameToRank = new HashMap<>();
+    private Map<String, ConferenceRankingEntry> acronymMap = new HashMap<>();
+    private Map<String, ConferenceRankingEntry> nameMap = new HashMap<>();


Use modern Java data structures. Maps should be initialized using Map.of() for empty maps instead of new HashMap<>(). This improves code readability and follows modern Java practices.

trag-bot · 2025-07-09T16:59:12Z

jablib/src/main/java/org/jabref/logic/icore/ICoreRankingRepository.java

+    private Map<String, ConferenceRankingEntry> acronymMap = new HashMap<>();
+    private Map<String, ConferenceRankingEntry> nameMap = new HashMap<>();
+    private StringSimilarity similarity = new StringSimilarity();
+    private Logger LOGGER = LoggerFactory.getLogger(ICoreRankingRepository.class);


Logger should be declared as private static final following Java naming conventions. Constants should be in uppercase, and loggers are typically static final members.

trag-bot · 2025-07-09T16:59:14Z

jablib/src/main/java/org/jabref/logic/icore/ICoreRankingRepository.java

+//            System.out.println("Loaded entries:");
+//            acronymToRank.forEach((key, val) -> System.out.println("Acronym: " + key + " -> " + val));
+//            nameToRank.forEach((key, val) -> System.out.println("Name: " + key + " -> " + val));
+        } catch (NullPointerException | IOException e) {


NullPointerException should not be caught as it indicates a programming error. Additionally, try block covers too much code. It should be split into smaller, more focused blocks.

trag-bot · 2025-07-09T16:59:47Z

jablib/src/main/java/org/jabref/logic/util/ConferenceUtil.java

+import java.util.regex.Pattern;
+
+public class ConferenceUtil {
+    public static Optional<String> extractAcronym(String title) {


Method parameter 'title' lacks null check, which could lead to NullPointerException when calling matcher. Method should validate input parameters to ensure robustness.

trag-bot · 2025-07-09T16:59:50Z

jablib/src/main/java/org/jabref/logic/util/ConferenceUtil.java

+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class ConferenceUtil {


Utility class should have a private constructor to prevent instantiation, following effective Java principles for utility classes containing only static methods.

trag-bot · 2025-07-09T17:00:18Z

jablib/src/main/java/org/jabref/model/entry/field/FieldFactory.java

@@ -231,7 +231,7 @@ private static Set<Field> getAllFields() {
     * separate preferences object
     */
    public static List<Field> getDefaultGeneralFields() {
-        List<Field> defaultGeneralFields = new ArrayList<>(Arrays.asList(StandardField.DOI, StandardField.CROSSREF, StandardField.KEYWORDS, StandardField.EPRINT, StandardField.URL, StandardField.FILE, StandardField.GROUPS, StandardField.OWNER, StandardField.TIMESTAMP));
+        List<Field> defaultGeneralFields = new ArrayList<>(Arrays.asList(StandardField.DOI, StandardField.ICORERANKING, StandardField.CROSSREF, StandardField.KEYWORDS, StandardField.EPRINT, StandardField.URL, StandardField.FILE, StandardField.GROUPS, StandardField.OWNER, StandardField.TIMESTAMP));


Using ArrayList with Arrays.asList is an outdated pattern. Modern Java provides more concise and readable alternatives like List.of() for creating immutable lists.

trag-bot · 2025-07-09T17:00:36Z

jablib/src/main/java/org/jabref/model/entry/field/StandardField.java

+    MODIFICATIONDATE("modificationdate", FieldProperty.DATE),
+    ICORERANKING("icoreranking");


The added field introduces unnecessary blank line before it, violating the consistent formatting pattern used in the enum declaration.

koppor · 2025-07-10T10:15:20Z

@asifebrahim Before requesting a review, please fix the tests. Although it seems, I have endless time, in reality, I don't. I like automation and that machines can support us all in making good code. Thus, please, look at the failing tests.

koppor · 2025-07-10T10:17:47Z

jablib/src/main/java/org/jabref/logic/util/ConferenceUtil.java

+
+public class ConferenceUtil {
+    public static Optional<String> extractAcronym(String title) {
+        Matcher matcher = Pattern.compile("\\((.*?)\\)").matcher(title);


Pattern needs to be a private static final variable to be really speed saving

jabref-machine · 2025-07-10T10:18:23Z

You committed your code on the main brach of your fork. This is a bad practice. The right way is to branch out from main, work on your patch/feature in that new branch, and then get that branch merged via the pull request (see GitHub flow).

For this pull request, this is OK. For subsequent pull requests, please start with a different branch with a proper branch name. See CONTRIBUTING.md for more details.

jabref-machine · 2025-07-10T10:18:23Z

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

koppor

Please adress my review comments - and also the commetns by trag-bot.

Expecially:

There are libraries for this. In JabRef, we use commons.apache.org/proper/commons-csv

trag-bot · 2025-07-10T10:18:45Z

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

jabref-machine · 2025-07-10T10:26:33Z

JUnit tests of jablib are failing. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Unit tests (pull_request)" and click on it.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Added the feature of IcoRanking

dc90043

trag-bot bot reviewed Jul 8, 2025

View reviewed changes

koppor requested changes Jul 9, 2025

View reviewed changes

Fixed all the unnecessary things

eb18adf

asifebrahim force-pushed the main branch from 8a94233 to eb18adf Compare July 9, 2025 16:49

asifebrahim requested a review from koppor July 9, 2025 16:51

trag-bot bot reviewed Jul 9, 2025

View reviewed changes

Merge branch 'main' into main

11738ef

koppor reviewed Jul 10, 2025

View reviewed changes

koppor requested changes Jul 10, 2025

View reviewed changes

		MODIFICATIONDATE("modificationdate", FieldProperty.DATE),
		ICORERANKING("icoreranking");

Uh oh!

Added the feature of IcoRanking #13512

Are you sure you want to change the base?

Added the feature of IcoRanking #13512

Conversation

asifebrahim commented Jul 8, 2025 • edited by Siedlerchr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

NOTE

Mandatory checks

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor commented Jul 9, 2025

Uh oh!

koppor commented Jul 9, 2025

Uh oh!

koppor commented Jul 9, 2025

Uh oh!

koppor commented Jul 9, 2025

Uh oh!

koppor commented Jul 9, 2025

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

asifebrahim commented Jul 8, 2025 •

edited by Siedlerchr

Loading