Skip to content

refactor: less objects during writing#15677

Merged
Siedlerchr merged 2 commits into
mainfrom
rich-bibtex
May 4, 2026
Merged

refactor: less objects during writing#15677
Siedlerchr merged 2 commits into
mainfrom
rich-bibtex

Conversation

@koppor

@koppor koppor commented May 4, 2026

Copy link
Copy Markdown
Member

Some small improvement on entry writing

Steps to test

Save a file

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • [/] I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@koppor koppor added the dev: code-quality Issues related to code or architecture decisions label May 4, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Reduce object allocation in bibtex entry writing

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Eliminate unnecessary TypedBibEntry object creation during entry writing
• Directly access entry type display name from BibEntry object
• Reduce memory allocation overhead in bibtex writing process
Diagram
flowchart LR
  A["BibEntry"] -->|"getType().getDisplayName()"| B["Type Display Name"]
  C["Removed: TypedBibEntry"] -.->|"no longer needed"| D["Memory Savings"]
  B --> E["BibWriter Output"]
Loading

Grey Divider

File Changes

1. jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java ✨ Enhancement +1/-2

Eliminate TypedBibEntry object creation

• Removed instantiation of TypedBibEntry object in writeEntryType() method
• Replaced with direct call to entry.getType().getDisplayName() for type display
• Maintains identical functionality while reducing object allocation

jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Unused mode parameter 🐞 Bug ⚙ Maintainability
Description
BibEntryWriter.writeEntryType still accepts BibDatabaseMode but no longer uses it after removing
TypedBibEntry, leaving a misleading/dead parameter. This increases maintenance risk because callers
may assume the mode affects how entry types are written when it currently does not.
Code

jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java[R146-148]

  private void writeEntryType(BibEntry entry, BibWriter out, BibDatabaseMode bibDatabaseMode) throws IOException {
      int start = out.getCurrentPosition();
-        TypedBibEntry typedEntry = new TypedBibEntry(entry, bibDatabaseMode);
-        out.write('@' + typedEntry.getTypeForDisplay());
+        out.write('@' + entry.getType().getDisplayName());
Evidence
writeEntryType still declares bibDatabaseMode but does not reference it in the body, and callers
still pass the mode through. TypedBibEntry#getTypeForDisplay confirms there is no mode-dependent
behavior for type display (it simply delegates to entry.getType().getDisplayName()).

jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java[96-152]
jablib/src/main/java/org/jabref/logic/bibtex/TypedBibEntry.java[16-56]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`BibEntryWriter#writeEntryType` takes a `BibDatabaseMode bibDatabaseMode` parameter, but after the refactor it is unused.
### Issue Context
`TypedBibEntry#getTypeForDisplay()` already returns `entry.getType().getDisplayName()` and does not use the mode, so keeping `bibDatabaseMode` in `writeEntryType` is now misleading and adds dead API surface.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java[96-152]
### What to change
- Remove the `bibDatabaseMode` parameter from `writeEntryType`.
- Update the call site in `writeRequiredFieldsFirstRemainingFieldsSecond` accordingly.
- Ensure there are no remaining unused imports/parameters after the signature change.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@Siedlerchr Siedlerchr enabled auto-merge May 4, 2026 20:58
@Siedlerchr Siedlerchr added this pull request to the merge queue May 4, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label May 4, 2026
Merged via the queue into main with commit 4bc13ca May 4, 2026
61 checks passed
@Siedlerchr Siedlerchr deleted the rich-bibtex branch May 4, 2026 21:38
Siedlerchr added a commit that referenced this pull request May 5, 2026
* upstream/main: (775 commits)
  Chore(deps): Bump com.konghq:unirest-modules-gson in /versions (#15682)
  Chore(deps): Bump org.glassfish.jaxb:jaxb-runtime in /versions (#15681)
  Update dependency com.konghq:unirest-modules-gson to v4.9.0 (#15679)
  Integrate with SearchRxiv  (#15373)
  Fix requirements (#15600)
  refactor: less objects during writing (#15677)
  Fix: suppress WARN for empty or blank column name in MainTableColumnModel#parse() (#15576)
  New Crowdin updates (#15676)
  Chore(deps): Bump com.github.ben-manes.caffeine:caffeine in /versions (#15673)
  Fix Nullwarnings - C (Mark bst package as nonnull by default) (#15663)
  Chore(deps): Bump com.github.javaparser:javaparser-symbol-solver-core (#15674)
  Chore(deps): Bump com.github.javaparser:javaparser-core in /versions (#15672)
  New Crowdin updates (#15669)
  Fix OpenRewrite (#15670)
  Udpate heylogs (and fix CHANGELOG.md) (#15671)
  Improve security and prevent shell injection for push2applications (#15628)
  Fix depdency analysis (#15668)
  Always use CI-local "gradle", instead of gradlew (#15667)
  Change OpenRewrite task to use rewriteDryRun (#15664)
  Add small documentation to parameter (#15666)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev: code-quality Issues related to code or architecture decisions status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants