improve find entry #1915
improve find entry #1915
Conversation
|
|
||
| public int findLastEntry(BibEntry entry) { | ||
| return model.getTableRows().lastIndexOf(entry); | ||
| EventList<BibEntry> tableRows = model.getTableRows(); |
There was a problem hiding this comment.
This can be replaced a combination of stream,filter and findFirst
sth like : tableRows.stream().filter(bibEntry->bibEntry.getId().equals(entry.getId()).findFirst()
And it has the advantage to return an optional which is way better to return a -1 value. Returning 1 comes from the old times of programming where everything >0 was indicating succes and negative things like -1 indicated errors. (Unfortunately I just can't find a source explaining it, but it is not considered good practice returning -1 nowadays if no element is found or error happend)
There was a problem hiding this comment.
I need to find the position of the given entry in the model.
With the statement you described you would get an Optional of an Entry.
There was a problem hiding this comment.
Ah yes, you are right. Currently there is no real good solution to return the index. :(
There was a problem hiding this comment.
I think in this particular case it is fine to use an "old school" for loop. But please rename i to row.
| @@ -19,6 +19,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# | |||
| - [#1897](https://github.com/JabRef/jabref/issues/1897) Implemented integrity check for `year` field: Last four nonpunctuation characters should be numerals | |||
|
|
|||
| ### Fixed | |||
| - Fixed selecting an entry out of multiple duplicates | |||
There was a problem hiding this comment.
Please add it at the end of the section.
There was a problem hiding this comment.
I try not to do that b/c then I have to rebase after nearly each merged PR.
|
Very small comments by my side, otherwise it looks good to me 👍 |
| EventList<BibEntry> tableRows = model.getTableRows(); | ||
| for (int row = 0; row < tableRows.size(); row++) { | ||
| BibEntry bibEntry = tableRows.get(row); | ||
| if (entry.getId().equals(bibEntry.getId())) { |
There was a problem hiding this comment.
I don't like that the ID is used to compare the entries (because I hope that we can get rid of the ID completely at some point). Does if (entry == bibentry) work?
boceckts
added
the
status: ready-for-review
|
|
koppor
removed
the
status: ready-for-review
I imroved the
findEntry-method some more. Now it always finds the correct Entry (by searching it with the id and not with the canonical representation.Steps to reproduce a situation in which the problem occurs