Open uli-f opened 1 year ago
@uli-f I agree with your proposal, it's better to fail early. I am not sure why RuntimeException
should be a concern, we are not implementing interface requiring CDKException
to worry about it.
Tests might need to be updated to throw RuntimeException
.
Okay. I made a PR for this change: PR #21
@ntk73 Would you please merge this one as well? Thanks.
Whether to fail early - and if so, how - might be something that I will discuss with the CDK developers once we open a PR to merge the rinchi code.
As a reminder for myself I would like to leave this open for now.
PR #21 is already merged.
I left a TODO for us in
RInChIGeneratorFactory
.RInChIGeneratorFactory
declared aCDKException
in its private constructor and stated in the documentation that this exception is thrown when the native library cannot be loaded. However, the native library wasn't loaded whenRInChIGeneratorFactory
was instantiated.InChIGeneratorFactory
does the same thing: declaring aCDKException
in its private constructor and stating in the documentation that this exception is thrown when the native library cannot be loaded. So my guess is that this is where this originates from.The code in
RInChIGeneratorFactory
is now changed due to my refactoring of the singleton pattern. However, the question remains: Shouldn'tRInChIGeneratorFactory
fail as early as possible if there is an issue with the loading of the native library?The following code loads the native library by explicitly loading the class io.github.dan2097.jnarinchi.JnaRinchi thereby making sure that there is an exception as soon as the the method
getInstance()
ofRInChIGeneratorFactory
is called:However, the main drawback is that the thrown exception must be a
RuntimeException
(or subclass thereof) as this happens in a static block.@ntk73 Any thoughts?