Simplify BibDatabaseWriter by removing unnecessary subclass #13408
Conversation
![trag-bot[bot]](https://avatars.githubusercontent.com/in/856808?s=60&v=4){kind=small}
| protected void writeString(BibtexString bibtexString, int maxKeyLength) throws IOException { | ||
| // If the string has not been modified, write it back as it was | ||
| if (!saveConfiguration.shouldReformatFile() && !bibtexString.hasChanged()) { | ||
| LOGGER.debug("Writing parsed serialization {}.", bibtexString.getParsedSerialization()); |
There was a problem hiding this comment.
Debug logging statement provides no additional value beyond what's obvious from the code. This violates the principle that comments/logs should add new information.
| // Write user comments | ||
| String userComments = bibtexString.getUserComments(); |
There was a problem hiding this comment.
Comment merely restates what the code does without providing additional context or reasoning. This type of trivial comment should be removed.
| // Writes the file encoding information. | ||
| bibWriter.write("% "); |
There was a problem hiding this comment.
Comment is redundant and simply restates what the code does. It doesn't provide any additional information or reasoning about why this is necessary.
subhramit
added
dev: code-quality
|
I went through the diff using RefactoringMinor: Looked good. Just the checkstyle thing and instantitation in JabKitLauncher to fix. |
koppor
added
the
dev: no-bot-comments
|
JBang is wrong on this one, code compiles fine, it looks like an incremental compilation issue on their side. |
Oh, the test checked out cd1c95a - which is not listed at https://github.com/JabRef/jabref/pull/13408/commits. The reason is The jar on maven central is not udpated - and there the "old" Writer is available. (In JabRef we accept breaking changes in SNAPSHOTS - and presumbly also accross versions; thus no worries ^^) |
The subclass created an unnecessary abstraction which made it harder to read the code since I had to switch back and forth between the parent and child classes. This could have been a reasonable tradeoff if we had multiple implementations of BibDatabaseWriter but we don't. Also, I don't think it makes sense anymore to have multiple implementations since we have Exporters
org.jabref.logic.exporter.Exporter.Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)