phenopackets / phenopacket-tools

An app and library for building, conversion, and validation of GA4GH Phenopackets.
http://phenopackets.org/phenopacket-tools/stable/
GNU General Public License v3.0
12 stars 5 forks source link

Introduce exception hierarchy #76

Closed ielis closed 1 year ago

ielis commented 2 years ago

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:

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

pnrobinson commented 2 years ago

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 1 year ago

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 1 year ago

Done