Smiles import minor adaptions #13
Conversation
…etected, the file name extended with the index of the line containing the SMILES code in the file (formerly extended with the count of so far parsed SMILES) is assigned as name to the structure; the ParsableLinesCount and InvalidLinesCount are now only logged if "invalid" lines were encountered;
| } catch (InvalidSmilesException | IndexOutOfBoundsException anException) { | ||
| //case: invalid line or SMILES code | ||
| //tmpContainsParsableSmilesCode stays false | ||
| } |
There was a problem hiding this comment.
Hm, I see, I probably shouldn't do that. Would you prefer this here? :
boolean tmpContainsParsableSmilesCode; //removed the initialization
try {
tmpProcessedLineArray = tmpSmilesFileNextLine.split(tmpSmilesFileDeterminedSeparator, 2);
if (!tmpProcessedLineArray[tmpSmilesCodeExpectedPosition].isEmpty()) {
tmpMolecule = tmpSmilesParser.parseSmiles(tmpProcessedLineArray[tmpSmilesCodeExpectedPosition]);
tmpContainsParsableSmilesCode = true;
tmpSmilesFileParsableLinesCounter++;
} else {
tmpContainsParsableSmilesCode = false; //new
}
} catch (InvalidSmilesException | IndexOutOfBoundsException anException) {
//case: invalid line or SMILES code
tmpContainsParsableSmilesCode = false; //new
}
There was a problem hiding this comment.
I guess the confusion comes from the expected SMILES position being empty (no exception) and the SMILES code being unparseable (exception) leading to the same code block which comes below. But yes, it is safer and more straightforward to understand to set these boolean values explicitly again in both cases, I'd say.
There was a problem hiding this comment.
That causes the confusion.
Anyway, I find it a bit unusual to place the logging outside the catch block. Why not like this and get rid of the Boolean:
try {
tmpProcessedLineArray = tmpSmilesFileNextLine.split(tmpSmilesFileDeterminedSeparator, 2);
String tmpSmiles = tmpProcessedLineArray[tmpSmilesCodeExpectedPosition].isBlank() ? null :
tmpProcessedLineArray[tmpSmilesCodeExpectedPosition];
tmpMolecule = tmpSmilesParser.parseSmiles(tmpSmiles);
tmpSmilesFileParsableLinesCounter++;
}
catch (InvalidSmilesException | IndexOutOfBoundsException anException) {
int tmpIndexInFile = tmpSmilesFileParsableLinesCounter + tmpSmilesFileInvalidLinesCounter;
Importer.LOGGER.info("Contains no parsable SMILES string: line " + tmpIndexInFile + " (index).");
tmpSmilesFileInvalidLinesCounter++;
continue;
}
This should cause an InvalidSmilesException if position in the file is blank or null.
|
Kudos, SonarCloud Quality Gate passed!
|
|
|
@FelixBaensch I've made the changes you requested. Please review again (merging is blocked otherwise). Thank you! |
Minor adaptions to the SMILES import: If no name for a structure is detected, the file name extended with the index of the line containing the SMILES code in the file (formerly extended with the count of so far parsed SMILES) is assigned as name to the structure; detected "invalid" lines are now being logged with their index in the file; the ParsableLinesCount and InvalidLinesCount are now only logged if "invalid" lines were encountered; added hyphen ("|") as possible SMILES file separator;
What do you two think of these changes?
Please review!