Context.getConceptService().getConcept(Integer.valueOf(Context.getAdministrationService().getGlobalProperty("concept.weight")));This is ugly for a few reasons:
- a helper method would save a lot of typing, and allow consistent reporting of error messages if the GP is configured wrong.
- proper unit-testing of any code that calls this requires you to use static mocks (thanks Mario and Alex for harping on this!)
Today Mark and I sat down (virtually) for a pairing session and wrote out our proposal for the "right way" to do this going forwards. Here's how it works in the EMR module:
- An EmrProperties class
- It has strongly-typed getter methods for the module's settings
- These getters are not static. The class is annotated with @Component, so we can autowire it into services that need it.
- This contains settings configurable via GP and hardcoded constants (so developers don't have to think about whether to look in a "properties" file or a "constants" file, and so if an initially-hardcoded setting is made configurable, this don't require structural refactoring)
- A ModuleProperties helper class, so other modules that depend on the EMR module (which will be most PIH modules going forwards) don't have to reimplement code that fetches an object from the database based on a global property.
- eventually we'll improve this to support PKs, UUIDs, names, etc.
The bolded point above is the key difference from things we've done before. So to use this class, we autowire an EmrProperties object (for production), providing a setter for testing. This lets us use the class in our code as conveniently as if we were making static calls, while letting us mock the EmrProperties object and inject it into our service, letting us write proper unit tests.