Modified the constructor for better handling #125
Conversation
Removed unnecessary 'order by 1 desc' clause from queries.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThese changes enhance error handling and data parsing robustness in two FHIR data model classes by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.javasrc/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java
🔇 Additional comments (3)
src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java (1)
76-81: LGTM!The method correctly propagates
FHIRExceptionand maintains proper null/empty array guard before delegating to the constructor.src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java (2)
63-65: Good defensive parsing for boolean field.The
isNationalparsing correctly handles multiple representations ("true","1") which is robust when dealing with heterogeneous data sources.
80-87: LGTM!The method correctly propagates
FHIRExceptionand maintains proper null/empty array guard before delegating to the constructor.
| try { | ||
|
|
||
| this.benVisitID = objArr[0] != null ? Long.parseLong(objArr[0].toString()) : null; | ||
| this.serviceProviderID = objArr[1] != null ? Short.parseShort(objArr[1].toString()) : null; | ||
| this.serviceProviderName = objArr[2] != null ? objArr[2].toString() : null; | ||
|
|
||
| this.benVisitID = objArr[0] != null ? (Long) objArr[0] : null; | ||
| this.serviceProviderID = objArr[1] != null ? (Short) objArr[1] : null; | ||
| this.serviceProviderName = objArr[2] != null ? (String) objArr[2] : null; | ||
| this.stateID = objArr[3] != null ? Integer.parseInt(objArr[3].toString()) : null; | ||
| this.stateName = objArr[4] != null ? objArr[4].toString() : null; | ||
|
|
||
| this.stateID = objArr[3] != null ? (Integer) objArr[3] : null; | ||
| this.stateName = objArr[4] != null ? (String) objArr[4] : null; | ||
| this.districtID = objArr[5] != null ? Integer.parseInt(objArr[5].toString()) : null; | ||
| this.districtName = objArr[6] != null ? objArr[6].toString() : null; | ||
|
|
||
| this.districtID = objArr[5] != null ? (Integer) objArr[5] : null; | ||
| this.districtName = objArr[6] != null ? (String) objArr[6] : null; | ||
| this.locationName = objArr[7] != null ? objArr[7].toString() : null; | ||
| this.address = objArr[8] != null ? objArr[8].toString() : null; | ||
|
|
||
| this.locationName = objArr[7] != null ? (String) objArr[7] : null; | ||
| this.address = objArr[8] != null ? (String) objArr[8] : null; | ||
| this.serviceID = objArr[9] != null ? Short.parseShort(objArr[9].toString()) : null; | ||
| this.serviceName = objArr[10] != null ? objArr[10].toString() : null; | ||
|
|
||
| this.serviceID = objArr[9] != null ? (Short) objArr[9] : null; | ||
| this.serviceName = objArr[10] != null ? (String) objArr[10] : null; | ||
| this.isNational = objArr[11] != null | ||
| ? objArr[11].toString().equalsIgnoreCase("true") || objArr[11].toString().equals("1") | ||
| : null; | ||
|
|
||
| this.isNational = objArr[11] != null ? (Boolean) objArr[11] : null; | ||
| this.abdmFacilityId = objArr[12] != null ? objArr[12].toString() : null; | ||
| this.abdmFacilityName = objArr[13] != null ? objArr[13].toString() : null; | ||
|
|
||
| this.abdmFacilityId = objArr[12] != null ? (String) objArr[12] : null; | ||
| this.abdmFacilityName = objArr[13] != null ? (String) objArr[13] : null; | ||
| this.psAddMapID = objArr[14] != null ? Integer.parseInt(objArr[14].toString()) : null; | ||
| this.providerServiceMapID = objArr[15] != null ? Integer.parseInt(objArr[15].toString()) : null; | ||
|
|
There was a problem hiding this comment.
Missing array bounds validation before element access.
The constructor accesses objArr[0] through objArr[15] (16 elements) without verifying that the array has sufficient length. An ArrayIndexOutOfBoundsException will be thrown for shorter arrays, which gets wrapped in a less informative FHIRException.
🔎 Proposed fix: Add bounds check at the start of try block
try {
+ if (objArr.length < 16) {
+ throw new FHIRException("Organization resource requires at least 16 elements, but got " + objArr.length);
+ }
this.benVisitID = objArr[0] != null ? Long.parseLong(objArr[0].toString()) : null;🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java
around lines 45 to 72, the constructor directly accesses objArr[0]..objArr[15]
without validating the array length; add a guard at the start of the try block
to verify objArr is not null and objArr.length >= 16 and if not, throw a
FHIRException (or return/handle) with a clear message indicating the missing
elements; keep the rest of the parsing logic the same so downstream parsing only
runs when the array has the expected 16 elements.
| } catch (Exception e) { | ||
| throw new FHIRException("Practioner resource failed with error - " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Incorrect error message: copy-paste error references "Practioner" instead of "Organization".
This will cause confusion during debugging since the exception message incorrectly identifies the resource type.
🔎 Proposed fix
- throw new FHIRException("Practioner resource failed with error - " + e.getMessage());
+ throw new FHIRException("Organization resource failed with error - " + e.getMessage());🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/data/resource_model/OrganizationDataModel.java
around lines 73 to 75, the catch block throws a FHIRException with a copy-paste
message referencing "Practioner" instead of "Organization"; update the message
to correctly reference "Organization" (and fix the "Practitioner" spelling if
needed) and include the original exception details (e.g., append e.getMessage()
or rethrow with e as the cause) so the thrown FHIRException accurately
identifies the resource and preserves error context.
| try { | ||
|
|
||
| this.benVisitID = objArr[0] != null ? Integer.parseInt(objArr[0].toString()) : null; | ||
| this.userID = objArr[1] != null ? Integer.parseInt(objArr[1].toString()) : null; | ||
|
|
||
| this.fullName = objArr[2] != null ? objArr[2].toString() : null; | ||
|
|
||
| this.dob = objArr[3] != null ? (objArr[3] instanceof Date ? (Date) objArr[3] : null) : null; | ||
|
|
||
| this.employeeID = objArr[4] != null ? objArr[4].toString() : null; | ||
| this.contactNo = objArr[5] != null ? objArr[5].toString() : null; | ||
| this.emailID = objArr[6] != null ? objArr[6].toString() : null; | ||
| this.qualificationName = objArr[7] != null ? objArr[7].toString() : null; | ||
| this.designationName = objArr[8] != null ? objArr[8].toString() : null; | ||
| this.genderName = objArr[9] != null ? objArr[9].toString() : null; | ||
|
|
||
| this.genderID = objArr[10] != null ? Integer.parseInt(objArr[10].toString()) : null; | ||
| this.serviceProviderID = objArr[11] != null ? Integer.parseInt(objArr[11].toString()) : null; | ||
|
|
||
| this.visitCode = objArr[12] != null ? Long.parseLong(objArr[12].toString()) : null; | ||
|
|
||
| this.createdBy = objArr[13] != null ? objArr[13].toString() : null; | ||
|
|
||
| this.createdDate = objArr[14] != null ? (objArr[14] instanceof Timestamp ? (Timestamp) objArr[14] : null) | ||
| : null; | ||
| } catch (Exception e) { | ||
| throw new FHIRException("Practioner resource failed with error - " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Missing array bounds validation before element access.
The constructor accesses objArr[0] through objArr[14] (15 elements) without verifying that the array has sufficient length. If a shorter array is passed, an ArrayIndexOutOfBoundsException will be thrown, which gets wrapped in a less informative FHIRException.
🔎 Proposed fix: Add bounds check at the start of try block
try {
+ if (objArr.length < 15) {
+ throw new FHIRException("Practitioner resource requires at least 15 elements, but got " + objArr.length);
+ }
this.benVisitID = objArr[0] != null ? Integer.parseInt(objArr[0].toString()) : null;🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java
around lines 44 to 71, the constructor indexes objArr[0]..objArr[14] without
verifying objArr is non-null and has at least 15 elements; add an explicit
pre-check at the start of the try block: if objArr is null or objArr.length <
15, throw a FHIRException with a clear message (e.g., "Invalid objArr: expected
at least 15 elements, got X") so we avoid ArrayIndexOutOfBoundsException and
provide a useful error; keep the existing parsing logic unchanged after the
guard.
| } catch (Exception e) { | ||
| throw new FHIRException("Practioner resource failed with error - " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Typo in error message: "Practioner" should be "Practitioner".
🔎 Proposed fix
- throw new FHIRException("Practioner resource failed with error - " + e.getMessage());
+ throw new FHIRException("Practitioner resource failed with error - " + e.getMessage());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (Exception e) { | |
| throw new FHIRException("Practioner resource failed with error - " + e.getMessage()); | |
| } | |
| } catch (Exception e) { | |
| throw new FHIRException("Practitioner resource failed with error - " + e.getMessage()); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wipro/fhir/data/resource_model/PractitionerDataModel.java
around lines 69 to 71, the exception message contains a typo ("Practioner");
update the thrown FHIRException message to use the correct spelling
"Practitioner" while preserving the rest of the message and concatenated error
details so the log remains informative.
|
1 participant
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.