Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor EntryEvents - removal part #5410
Refactor EntryEvents - removal part #5410
Changes from 12 commits
80e8377
48cb7fd
923b40e
a0ffcb5
b08ae88
3a830a8
4e57a46
b990e0d
3122e67
aac840c
90c27f1
a0cafc4
868deed
2afb868
460f44a
f11389e
f535a0b
77ff450
b0805cc
7ac7a1c
8255ad2
3b8dfea
5f1b811
c41dc17
bb88484
4c97c64
30d8d51
5cf5965
1df2869
0f5d76f
13abf0c
17f04b0
214e958
a90c44e
648c5da
b0f585a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the constructor of
UndoableRemoveEntries
accepting a single BibEntry, and similarly the methodBibDatabase#removeEntry
.(Btw, there is
Colections.singletonList(element)
which is preferred overnew ArrayList<>(element)
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that
DuplicateSearch.getToRemove
returns a Collection, not a single one. The only way I found that I could coerce aCollection
to aList
was via theArrayList<>
constructor. I could make a constructor for aCollection
, but that feels to specific.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the second constructor. Even if very specific, the code with
new ArrayList<>
reads strange.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collections.singletonList
should work here, doesn't it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe
DuplicateSearch.DuplicateSearchResult.getToRemove
returns aCollection
, and it could have multiple elements. As of now, I am simply coercing it into aList
. However, it might make more sense to do the coercion in the methodDuplicateSearch.DuplicateSearchResult.getToRemove
so that it matchesgetToAdd
, which returns aList
. Let me know your thoughts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made the change, as I think it's better if
getToRemove
returns aList
.This file was deleted.