Check integrity edition check implemented #1914
Conversation
|
Please set label "ready-for-review". Thank you. |
Siedlerchr
added
the
status: ready-for-review
| return Collections.emptyList(); | ||
| } | ||
| //BibTeX | ||
| if (!FIRST_LETTER_CAPITALIZED.test(value.get().trim()) && (!bibDatabaseContext.isBiblatexMode())) { |
There was a problem hiding this comment.
I would have changed the order of the statements in the conditions, checking for biblatex mode first:
Because the && is a short circuit operator, so the regex would only get evaluated if the mode condition is true
Edit// But as the regex is compiled I think the order doesn't really matter.
|
LGTM +1, Just let someone other look at it, too. Another thing I just got in mind is that the whole checker code could be refactored to use lamdbas, because we have a nice Functional Interface. Will create a new issue for discussing that. |
| @@ -28,7 +28,7 @@ | |||
|
|
|||
| public class IntegrityCheck { | |||
|
|
|||
| private final BibDatabaseContext bibDatabaseContext; | |||
| private static BibDatabaseContext bibDatabaseContext; | |||
There was a problem hiding this comment.
why do you want this to be static?
There was a problem hiding this comment.
Because I want to use a method of this class in class EditionChecker. An alternative is, to define a objectprivate final BibDatabaseContext bibDatabaseContext; in class EditionChecker, but this is redundant.
There was a problem hiding this comment.
. An alternative is, to define a private final BibDatabaseContext
bibDatabaseContext; in class EditionChecker,
Seems to be the better solution as static variables sound like global
variables...
There was a problem hiding this comment.
Many FieldChecker need access to the isBiblatexMode() method. It is bad to create for every FieldChecker class a new bibDatabaseContext object and inizialize it (in the FieldChecker constructor?).
There was a problem hiding this comment.
What about passing the bibDatabaseContext in the constructor of the Field Checker?
Edit// Even better, just pass a boolean in the constructor holding the value of isBiblatexMode..
|
LGTM 👍, too. Just change the order in the if statement as @Siedlerchr suggest and then this could be merged. |
|
And please extract the checker to its own file. |
|
LGTM from my side now. 👍 |
|
@grimes2 Please rebase this on master and we will merge it in. |
* Mark some methods as deprecated in BibEntry and BibDatabase * Rename getResolvedFieldOrAlias * Use flatmap
* Fix location field not exported to office 2007 xml * Add some test for exporting location and address field Address in xml field is now imported as location add some javadoc
* Add possibility to remember password. - Add checkbox to the open shared database dialog - Add SharedDatabasePreferences - Add password encrypting and decrypting methods - Update localization entries - Reorganize clearing methods for Preferences * Change prefs node and add class comment. * Relativate node path for password storage. * Fix LOGGER factory. * Improve password encryption using XOR. - Use username as one operand - Add new parameter to the constructor * Extract method. * Improve exception handling. * Improve password encryption and decryption. - Use CBC and padding - Hash the key before using - Simplify conversion * Fix modifier. Fix conflicts.
| } | ||
|
|
||
| //BibLaTeX | ||
| if (!ONLY_NUMERALS_OR_LITERALS.test(value.get().trim()) && (bibDatabaseContextEdition.isBiblatexMode())) { |
There was a problem hiding this comment.
Just a minor thing: if you change the order of the test and mode it will be slightly faster (since checking mode is faster and the test will not happen is the mode is wrong).
Not required for merge though.
|
👍 |
|
Thanks for your contribution! |
#1912 Implemented a check for the field edition. The check differentiates between BibTeX and BibLaTeX mode of the database.