Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor duplicate import logic in ImportController #1115

Open
rashi07dashore opened this issue Jun 18, 2024 · 3 comments
Open

Refactor duplicate import logic in ImportController #1115

rashi07dashore opened this issue Jun 18, 2024 · 3 comments

Comments

@rashi07dashore
Copy link

1. Section, tab, or page affected:

This issue applies to the ImportController class and specifically the methods importOrganizations and importProviders.

2. Steps to reproduce:

Access the OpenELIS application.
Navigate to any functionality that utilizes the ImportController for importing data (This information might require further investigation).
Observe that the importOrganizations and importProviders methods contain duplicate logic for fetching the relevant service and calling the import method.

3. Severity:

  • This issue is something we would like to see improved in the future but does not significantly affect our work.

While the current functionality works, refactoring the code improves readability and maintainability, especially if additional import functionalities are implemented in the future.

### Detailed Description:
The current implementation of importOrganizations and importProviders methods contain duplicate logic for fetching the service and calling the import method. This makes the code less readable and harder to maintain.

Here's the current code snippet:
@GetMapping(value = "/organization")
public void importOrganizations() throws FhirLocalPersistingException, FhirGeneralException, IOException {
SpringContext.getBean(OrganizationImportService.class).importOrganizationList();
}

@GetMapping(value = "/provider")
public void importProviders() throws FhirLocalPersistingException, FhirGeneralException, IOException {
SpringContext.getBean(ProviderImportService.class).importPractitionerList();
}

### Proposed Solution
Extract the common logic into a separate private method to improve readability and maintainability. Here's an example of the refactored code:

private void importDataFromFhir(String fhirResourceType) throws FhirLocalPersistingException, FhirGeneralException, IOException {
// Get the appropriate service based on resource type
ImportService importService;
if (fhirResourceType.equals("organization")) {
importService = SpringContext.getBean(OrganizationImportService.class);
} else if (fhirResourceType.equals("provider")) {
importService = SpringContext.getBean(ProviderImportService.class);
} else {
// Handle invalid resource type (optional)
}

// Call the import method based on the service
importService.importList(fhirResourceType); // Assuming a generic importList method
}
@GetMapping(value = "/organization")
public void importOrganizations() throws FhirLocalPersistingException, FhirGeneralException, IOException {
importDataFromFhir("organization");
}

@GetMapping(value = "/provider")
public void importProviders() throws FhirLocalPersistingException, FhirGeneralException, IOException {
importDataFromFhir("provider");
}

This approach promotes code reuse and simplifies future modifications if additional resource types are introduced.

@Bahati308
Copy link
Contributor

hello @rashi07dashore , can i work on this?

@Ariho-Seth
Copy link

hello @Bahati308 how far with this issue? Did you finish working on it?

@Bahati308
Copy link
Contributor

yes @Ariho-Seth , you can take it up. No worries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants