Report Testing examples for rwandareports

[rubailly]

No description provided.

[mseaton]

I'm not personally a huge fan of classes that start with "_". Not sure why Rowan does this, but worth checking with him. I'd also probably also make such things enums rather than public static final classes.

[rowanseymour]

The _ convention is to avoid name conflicts with the actual classes. Enums would buy you some type safety but then it would be harder to implement a single MetadataUtils.getLocation(...) method. You'd have to find a way to associate the enum value with the UUID, and you'd have the problem of CommonMetadata._Location being a different class to HivMetadata._Location etc

[mseaton]

I had originally assumed this was the reason. But - maybe I'm missing something - but HivMetadata doesn't inherit from CommonMetadata, so I'm not sure how HivMetadata would expose the same ._Location class as the one used in CommonMetadata. I get your point with enums, though enums can implement interfaces, so you could do something like this:

public interface LocationBuilder {
String getUuid();
String getName();
String getDescription();
LocationBuilder(String uuid, String name, String description) { ... }
}

public enum CoreLocations implements LocationBuilder {
UNKNOWN_LOCATION("8d6c993e-c2cc-11de-8d13-0010c6dffd0f", "Unknown Location", "An unknown location");
}

public enum HivLocations implements LocationBuilder {
HIV_CLINIC_1("1b6c993e-c2cc-11de-8d13-0010c6dffd0f", "Hiv Clinic 1", "The first HIV clinic"),
HIV_CLINIC_2("2c6c993e-c2cc-11de-8d13-0010c6dffd0f", "Hiv Clinic 2", "The second HIV clinic"),
}

Then, in your install tools can do something like:

public void install(LocationBuilder locationBuilder) {
Location location = new Location();
location.setUuid(locationBuilder.getUuid());
location.setName(locationBuilder.getName());
location.setDescription(locationBuilder.getDescription());
locationService.saveLocation(location);
}

Thoughts?

[rowanseymour]

What I meant about name conflicts is that HivMetadata.Location conflicts with org.openmrs.Location.

I like your enum idea because it means that metadata identification isn't separate from description. Not entirely sure though how MetadataUtils.getLocation(LocationBuilder builder) {} would work

[mseaton]

I assume it would just do:
return Context.getLocationService().getLocationByUuid(builder.getUuid());

[rowanseymour]

I've done a bit of experimenting with this approach but it gets verbose very quickly for one reason: Java doesn't support Enum super classes. So every builder class has to re-implement all the builder interface methods. It also doesn't lend itself to supporting different ways of deploying metadata. Currently bundles are completely agnostic about how metadata objects are created so you can implement whatever custom logic you need. In KenyaEMR, our location metadata bundle actually synchronises against a CSV list of locations. String constants aren't ideal but they are simple to work with.

[mseaton]

I suppose this could potentially be handled via injection though, right? i.e. instead of having a class hierarchy that provides these base methods, add another property on the enum that is a "Class<? extends MetadataBuilder> builder". Then there would just be a common method shared by all that simply instantiates a new builder and passes itself into the "build(...)" method. Possibility?

[rowanseymour]

Anything is a possibility but currently String constants are working just fine :) Consider also that the MetadataUtils.getX() methods all throw exceptions if the requested item doesn't exist, so even if you did manage to type something like MetadataUtils.getLocation(HivMetadata._Form.REGISTRATION), you'd get a fairly clear exception right away.