phenopackets/phenopacket-tools

Introduce exception hierarchy

ielis opened this issue · 3 comments

ielis commented

Should we introduce an exception hierarchy into the library?

Ideally, there should be a parent PhenopacketToolsRuntimeException, and all phenopacket-tools exceptions should extend the parent. This would allow writing catch-all constructs in the client code to prevent "explosions". For instance:

try {
  Age a = Ages.age("P1Y10N").build();
} catch (PhenopacketToolsRuntimeException e) {
  // sorry, 'N' is invalid
} 

However, we do not currently have a suitable module for PhenopacketToolsRuntimeException that is shared by all base modules:

  • phenopacket-tools-builder
  • phenopacket-tools-converter
  • phenopacket-tools-validator-core.

The PhenotoolsRuntimeException that resides in phenopacket-tools-builder is a good candidate for either inheriting from PhenopacketToolsRuntimeException or for being replaced.

Probably a good idea. I am not convinced that we want to have a module for this though, and wonder if it would be better to have the modules
core
builder
converter
validator
cli
where the last four modules depend on core and cli depends on all.
The validation should always be JSON and Message based.

ielis commented

The #110 introduces the core module with base exceptions and several other constants that are used by all other modules.

The PR provides PhenopacketToolsException and PhenopacketToolsRuntimeException as base checked and unchecked exceptions. The base exceptions are extended by the other phenopacket-tools exceptions, which makes it super easy to catch all exceptions by the calling code.

This issue can be closed by #110

ielis commented

Done