Skip to content

Commit

Permalink
Use BibEntry.getId instead of System.identityHashCode
Browse files Browse the repository at this point in the history
  • Loading branch information
LoayGhreeb committed Aug 29, 2024
1 parent 2310625 commit f369b09
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
title: Use System.identityHashCode for BibEntry at indexing
title: Use BibEntry.getId for BibEntry at indexing
nav_order: 38
parent: Decision Records
---

<!-- markdownlint-disable-next-line MD025 -->
# Use `System.identityHashCode` for BibEntries at Indexing
# Use `BibEntry.getId` for BibEntries at Indexing

## Context and Problem Statement

Expand All @@ -22,9 +22,9 @@ This, however, is not useful in the UI, where equal entries are not the same ent

## Considered Options

* Use `System.identityHashCode` for indexing `BibEntry`
* Use `BibEntry.getId` for indexing `BibEntry`
* Rewrite `BibEntry` logic

## Decision Outcome

Chosen option: "Use `System.identityHashCode` for indexing `BibEntry`", because is the "natural" thing to ensure distinction between two instances of a `BibEntry` object - regardless of equality.
Chosen option: "Use `BibEntry.getId` for indexing `BibEntry`", because is the "natural" thing to ensure distinction between two instances of a `BibEntry` object - regardless of equality.
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ private void handleDuplicates(DuplicateSearchResult result) {
*/
static class DuplicateSearchResult {

private final Map<Integer, BibEntry> toRemove = new HashMap<>();
private final Map<String, BibEntry> toRemove = new HashMap<>();
private final List<BibEntry> toAdd = new ArrayList<>();

private int duplicates = 0;
Expand All @@ -234,7 +234,7 @@ public synchronized List<BibEntry> getToAdd() {

public synchronized void remove(BibEntry entry) {
// ADR-0038
toRemove.put(System.identityHashCode(entry), entry);
toRemove.put(entry.getId(), entry);
duplicates++;
}

Expand All @@ -252,7 +252,7 @@ public synchronized void replace(BibEntry entry, BibEntry replacement) {

public synchronized boolean isToRemove(BibEntry entry) {
// ADR-0038
return toRemove.containsKey(System.identityHashCode(entry));
return toRemove.containsKey(entry.getId());
}

public synchronized int getDuplicateCount() {
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class GroupNodeViewModel {
private final BibDatabaseContext databaseContext;
private final StateManager stateManager;
private final GroupTreeNode groupNode;
private final ObservableMap<Integer, BibEntry> matchedEntries = FXCollections.observableHashMap();
private final ObservableMap<String, BibEntry> matchedEntries = FXCollections.observableHashMap();
private final SimpleBooleanProperty hasChildren;
private final SimpleBooleanProperty expandedProperty = new SimpleBooleanProperty();
private final BooleanBinding anySelectedEntriesMatched;
Expand Down Expand Up @@ -258,21 +258,21 @@ private void onDatabaseChanged(ListChangeListener.Change<? extends BibEntry> cha
for (BibEntry changedEntry : change.getList().subList(change.getFrom(), change.getTo())) {
if (groupNode.matches(changedEntry)) {
// ADR-0038
matchedEntries.put(System.identityHashCode(changedEntry), changedEntry);
matchedEntries.put(changedEntry.getId(), changedEntry);
} else {
// ADR-0038
matchedEntries.remove(System.identityHashCode(changedEntry));
matchedEntries.remove(changedEntry.getId());
}
}
} else {
for (BibEntry removedEntry : change.getRemoved()) {
// ADR-0038
matchedEntries.remove(System.identityHashCode(removedEntry));
matchedEntries.remove(removedEntry.getId());
}
for (BibEntry addedEntry : change.getAddedSubList()) {
if (groupNode.matches(addedEntry)) {
// ADR-0038
matchedEntries.put(System.identityHashCode(addedEntry), addedEntry);
matchedEntries.put(addedEntry.getId(), addedEntry);
}
}
}
Expand Down Expand Up @@ -300,7 +300,7 @@ private void updateMatchedEntries() {
.onSuccess(entries -> {
matchedEntries.clear();
// ADR-0038
entries.forEach(entry -> matchedEntries.put(System.identityHashCode(entry), entry));
entries.forEach(entry -> matchedEntries.put(entry.getId(), entry));
})
.executeWith(taskExecutor);
}
Expand Down Expand Up @@ -549,10 +549,10 @@ public void listen(IndexAddedOrUpdatedEvent event) {
searchGroup.updateMatches(entry);
if (groupNode.matches(entry)) {
// ADR-0038
matchedEntries.put(System.identityHashCode(entry), entry);
matchedEntries.put(entry.getId(), entry);
} else {
// ADR-0038
matchedEntries.remove(System.identityHashCode(entry));
matchedEntries.remove(entry.getId());
}
}
}).executeWith(taskExecutor);
Expand All @@ -566,7 +566,7 @@ public void listen(IndexRemovedEvent event) {
for (BibEntry entry : event.entries()) {
searchGroup.updateMatches(entry);
// ADR-0038
matchedEntries.remove(System.identityHashCode(entry));
matchedEntries.remove(entry.getId());
}
}).executeWith(taskExecutor);
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/jabref/model/groups/SearchGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class SearchGroup extends AbstractGroup {
public static final Version VERSION_6_0_ALPHA = Version.parse("6.0-alpha");
private static final Logger LOGGER = LoggerFactory.getLogger(SearchGroup.class);

private final ObservableMap<Integer, BibEntry> matchedEntries = FXCollections.observableHashMap();
private final ObservableMap<String, BibEntry> matchedEntries = FXCollections.observableHashMap();
private SearchQuery query;
private LuceneManager luceneManager;

Expand Down Expand Up @@ -75,7 +75,7 @@ public void updateMatches() {
matchedEntries.clear();
// TODO: Search should be done in a background thread
// ADR-0038
luceneManager.search(query).getMatchedEntries().forEach(entry -> matchedEntries.put(System.identityHashCode(entry), entry));
luceneManager.search(query).getMatchedEntries().forEach(entry -> matchedEntries.put(entry.getId(), entry));
}

public void updateMatches(BibEntry entry) {
Expand All @@ -84,10 +84,10 @@ public void updateMatches(BibEntry entry) {
}
if (luceneManager.isMatched(entry, query)) {
// ADR-0038
matchedEntries.put(System.identityHashCode(entry), entry);
matchedEntries.put(entry.getId(), entry);
} else {
// ADR-0038
matchedEntries.remove(System.identityHashCode(entry));
matchedEntries.remove(entry.getId());
}
}

Expand All @@ -108,7 +108,7 @@ public boolean equals(Object o) {
@Override
public boolean contains(BibEntry entry) {
// ADR-0038
return matchedEntries.containsKey(System.identityHashCode(entry));
return matchedEntries.containsKey(entry.getId());
}

@Override
Expand Down

0 comments on commit f369b09

Please sign in to comment.