Fixes RIS' data field

[victorjof]

This pull resquest fixes issue #4816.

When importing RIS files Jabref checks for date fields (Y1,PY,DA and Y2) and uses the first one it finds. The problem is that Y2 refers to access date, DA in most cases is a generic date, and PY is the publication year(reliable field)*.

jabref/src/main/java/org/jabref/logic/importer/fileformat/RisImporter.java

Lines 196 to 203 in 40600b3

	} else if (!foundDate && (("Y1".equals(tag) || "Y2".equals(tag) || "PY".equals(tag) || "DA".equals(tag)) && (value.length() >= 4))) {
	String year = value.substring(0, 4);
	try {
	Year.parse(year, formatter);
	//if the year is parsebale we have found our date
	fields.put(FieldName.YEAR, value.substring(0, 4));
	foundDate = true;

We couldn't find references to Y1('primary date'?). However, as already cited in #4816, and by doing some exports we found Y1 to be PY's synonym, furthermore, when PY received a higher priority than Y1, some tests cases didn't pass as there were a missing month field in .bib.

We established a priority system indicated by the dateTags (lowest index higher priority)

https://github.com/VMichelan/jabref/blob/2f243c16f08f2cfeba6f1ab4651970d90eb55546/src/main/java/org/jabref/logic/importer/fileformat/RisImporter.java#L31

We added 2 test cases for this and changed one of the old ones adding the month field.

*https://web.archive.org/web/20120717122530/http://refman.com/support/direct%20export.zip
** nicely displayed in: https://github.com/aurimasv/translators/wiki/RIS-Tag-Map

---

• Change in CHANGELOG.md described
• Tests created for changes
• Manually tested changed features in running JabRef
• Screenshots added in PR description (for bigger UI changes)
• Ensured that the git commit message is a good one
• Check documentation status (Issue created for outdated help page at help.jabref.org?)

[Siedlerchr]

Thanks for your contribution! 👍
Only some minor code improvements, but in general it looks fine. and the tests are also a good thing!

[VMichelan]

Thanks for the feedback @Siedlerchr!

Those are great suggestions, using a List is much better, and since indexOf returns -1 if it doesn't find the element, we can use it to find out if the element is in dateTags and to get its index like this:

} else if ((tagPriority = dateTags.indexOf(tag)) != -1 && value.length() >= 4) {

    if (tagPriority < datePriority) {

I think this is a good solution, the problem is that like isDateTag it's also O(n), I don't know if that's really a problem, but if it is maybe we should use a HashMap?

Also, since finding out if a tag is a date is a one liner now, I will remove the isDateTag and getDatePriority methods, and because we don't need to pass dateTags to these methods anymore I think we should move it from the class to the method importDatabase.

I would like to hear what you think about these considerations, I think with this out of the way we can make the final version and fix the failed check to make it ready for merge, thanks.

[Siedlerchr]

I have an even better idea, you can omit one search in the list,
If you directly use the indexOf before the if/else cases and init it e.g. next to the month initialisation
you can then simply reuse the value here if it's <0

[VMichelan]

I coudn't figure out what you meant here, the month initialization is outside of the loop

            Optional<Month> month = Optional.empty();
            Map<String, String> fields = new HashMap<>();

            String[] lines = entry1.split("\n");

            for (int j = 0; j < lines.length; j++) {

but the tag variable is only declared and used inside of the loop, so I couldn't use indexOf(tag) at this point in the code, so I kept the if statement that way.

[Siedlerchr]

If your ArrayList is ordered/sorted, you can use Collections,binarySearch, but I think in this case it's over engineering ;)

Premature optimization is the root of all evil ~ Donald Knuth

The complexity really is only for large N.
But I think you can omit one time searching the list, see my comment at the code.

[Siedlerchr]

Thanks for the quick follow up, looks good now!

[koppor]

Thank you for your work!