Hello!
methods in openmrs-core service implementation have a tendency to be long and complex, they are hard to read and thus maintain.
I wonder if you would be in favor of refactorings such as the following:
take PatientServiceImpl.savePatient(Patient)
it does a lot of things. If we would extract the related parts into smaller private methods we have the opportunity to give longer meaningful names and turn it into something like this:
public Patient savePatient(Patient patient) throws APIException {
requireAppropriatePatientModificationPrivilege(patient);
if (!patient.isVoided()) {
checkPatientIdentifiers(patient);
}
setPreferredPatientIdentifier(patient);
setPreferredPatientName(patient);
setPreferredPatientAddress(patient);
return dao.savePatient(patient);
}
and Im sure by doing that we can also simplify the code we extracted into the private methods since we’ll see things more clearly as the methods would then do less.
I know touching code is always a risk, but we do trust our tests, right
I think such improvements will make it easier for everyone to read the code and thus maintain it.
Should I create issues related to such refactorings? What do you say?