Added all null checks for fhir bundle code #100
Conversation
WalkthroughNull checks and safer casting have been added to the constructors of multiple data model classes, ensuring that fields are only assigned if the corresponding input array elements are non-null. Numeric conversions now use Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DataModel
Caller->>DataModel: new DataModel(Object[] objArr)
alt For each field in DataModel
DataModel->>DataModel: if objArr[i] != null assign/cast value
else DataModel->>DataModel: assign null
end
Caller-->>DataModel: Receives safely constructed object
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (4)
src/main/java/com/wipro/fhir/data/resource_model/VitalsAnthropometryModel.java (4)
62-96: 🛠️ Refactor suggestionAdd array bounds validation to prevent ArrayIndexOutOfBoundsException.
The constructor assumes the input array has at least 19 elements but doesn't validate this assumption. This could lead to runtime exceptions if a smaller array is passed.
Add array length validation at the beginning of the constructor:
public VitalsAnthropometryModel(Object[] objArr) { + if (objArr == null || objArr.length < 19) { + throw new IllegalArgumentException("Input array must contain at least 19 elements"); + } if (objArr[0] != null)
62-96: 🛠️ Refactor suggestionAdd array bounds checking for robustness.
The constructor assumes the input array has at least 19 elements but doesn't verify this assumption. This could lead to
ArrayIndexOutOfBoundsException.Consider adding array length validation:
public VitalsAnthropometryModel(Object[] objArr) { + if (objArr == null || objArr.length < 19) { + throw new IllegalArgumentException("Input array must have at least 19 elements"); + } if (objArr[0] != null)
93-94: 🛠️ Refactor suggestionAdd null checks for remaining fields.
These fields lack null checks, creating inconsistency with the defensive programming approach applied to other fields in this constructor.
Apply null checks consistently:
- this.createdDate = (Timestamp) objArr[17]; - this.createdBy = (String) objArr[18]; + this.createdDate = objArr[17] != null ? (Timestamp) objArr[17] : null; + this.createdBy = objArr[18] != null ? (String) objArr[18] : null;
62-96: 🛠️ Refactor suggestionAdd array bounds validation.
The constructor assumes
objArrhas at least 19 elements but doesn't validate this, which could causeArrayIndexOutOfBoundsException.Add validation at the beginning of the constructor:
public VitalsAnthropometryModel(Object[] objArr) { + if (objArr == null || objArr.length < 19) { + throw new IllegalArgumentException("Invalid input array"); + }
♻️ Duplicate comments (1)
src/main/java/com/wipro/fhir/data/resource_model/AllergyIntoleranceDataModel.java (1)
63-76: Consistent null safety implementation - same recommendations apply.The implementation follows the same pattern as other data models, which is good for consistency. The same potential improvements mentioned for
AppointmentDataModelapply here:
- Add array bounds checking for the expected 12 elements
- Consider additional type validation for numeric conversions
- Validate that
objArritself is not nullThe consistent approach across data models makes the codebase more maintainable.
🧹 Nitpick comments (8)
src/main/java/com/wipro/fhir/data/resource_model/AppointmentDataModel.java (1)
63-65: Consider wrapping numeric conversions in try-catch for better error handling.While the
Numbercasting approach is safer than direct primitive casting, it can still throwClassCastExceptionif the array element is not a numeric type.-this.id = objArr[0] != null ? BigInteger.valueOf(((Number) objArr[0]).longValue()) : null; +this.id = objArr[0] != null ? + (objArr[0] instanceof Number ? BigInteger.valueOf(((Number) objArr[0]).longValue()) : null) : null;Or use a helper method for safer conversion:
private BigInteger safeConvertToBigInteger(Object obj) { if (obj == null) return null; if (obj instanceof Number) { return BigInteger.valueOf(((Number) obj).longValue()); } return null; // or throw exception based on requirements }src/main/java/com/wipro/fhir/data/resource_model/LabTestAndComponentModel.java (1)
80-81: BigDecimal casting requires the same safety considerations.The
BigDecimalcasting on lines 80-81 has the same potentialClassCastExceptionrisk as the numeric conversions.Consider type checking for
BigDecimalfields as well:-this.rangeMin = objArr[10] != null ? (BigDecimal) objArr[10] : null; -this.rangeMax = objArr[11] != null ? (BigDecimal) objArr[11] : null; +this.rangeMin = objArr[10] != null && objArr[10] instanceof BigDecimal ? (BigDecimal) objArr[10] : null; +this.rangeMax = objArr[11] != null && objArr[11] instanceof BigDecimal ? (BigDecimal) objArr[11] : null;src/main/java/com/wipro/fhir/data/resource_model/EncounterDataModel.java (1)
63-65: Ensure consistent error handling for both longValue() and intValue() conversions.Lines 63-65 use
intValue()while other models primarily uselongValue(). Both have the sameClassCastExceptionrisk if the object is not aNumber.Consider creating a common utility method for safe numeric conversions:
private Integer safeConvertToInteger(Object obj) { if (obj == null) return null; if (obj instanceof Number) { return ((Number) obj).intValue(); } return null; // or handle according to business requirements }src/main/java/com/wipro/fhir/data/resource_model/ConditionDiagnosisDataModel.java (1)
59-67: Excellent null safety implementation with room for additional defensive programming.The null checks effectively prevent
NullPointerExceptionand theNumbercasting approach forBigIntegerfields is safer than directlongcasting. This handles various numeric types gracefully.Consider adding array validation for even more robust defensive programming:
public ConditionDiagnosisDataModel(Object[] objArr) { + if (objArr == null || objArr.length < 9) { + throw new IllegalArgumentException("Object array must not be null and must contain at least 9 elements"); + } this.id = objArr[0] != null ? BigInteger.valueOf(((Number) objArr[0]).longValue()) : null; this.beneficiaryRegID = objArr[1] != null ? BigInteger.valueOf(((Number) objArr[1]).longValue()) : null; this.visitCode = objArr[2] != null ? BigInteger.valueOf(((Number) objArr[2]).longValue()) : null; this.providerServiceMapID = objArr[3] != null ? (Integer) objArr[3] : null; this.vanID = objArr[4] != null ? (Integer) objArr[4] : null; this.sctTerm = objArr[5] != null ? (String) objArr[5] : null; this.sctcode = objArr[6] != null ? (String) objArr[6] : null; this.createdDate = objArr[7] != null ? (Timestamp) objArr[7] : null; this.createdBy = objArr[8] != null ? (String) objArr[8] : null; }src/main/java/com/wipro/fhir/data/resource_model/VitalsAnthropometryModel.java (1)
64-69: Consider using safer numeric casting approach.The current approach casts directly to
long, which could throwClassCastExceptionif the array element is a different numeric type. Other files in this PR use a safer approach.Consider using the safer casting pattern used in other files:
- if (objArr[0] != null) - this.beneficiaryRegID = BigInteger.valueOf((long) objArr[0]); - if (objArr[2] != null) - this.visitCode = BigInteger.valueOf((long) objArr[2]); + if (objArr[0] != null) + this.beneficiaryRegID = BigInteger.valueOf(((Number) objArr[0]).longValue()); + if (objArr[2] != null) + this.visitCode = BigInteger.valueOf(((Number) objArr[2]).longValue());src/main/java/com/wipro/fhir/data/resource_model/FamilyMemberHistoryDataModel.java (1)
59-70: Consider adding array bounds validation.While the null checks are comprehensive, the constructor should also validate that the input array has the expected number of elements.
Add validation:
public FamilyMemberHistoryDataModel(Object[] objArr) { + if (objArr == null || objArr.length < 10) { + throw new IllegalArgumentException("Invalid input array"); + }src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java (1)
81-106: Add array bounds validation for robustness.Consider validating the input array length to prevent
ArrayIndexOutOfBoundsException.Add validation:
public MedicationRequestDataModel(Object[] objArr) { + if (objArr == null || objArr.length < 19) { + throw new IllegalArgumentException("Invalid input array"); + }src/main/java/com/wipro/fhir/data/resource_model/DiagnosticReportDataModel.java (1)
69-89: Consider array bounds validation.The implementation is robust for null handling but should also validate array length to prevent potential
ArrayIndexOutOfBoundsException.Add validation:
public DiagnosticReportDataModel(Object[] objArr) { + if (objArr == null || objArr.length < 18) { + throw new IllegalArgumentException("Invalid input array"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/wipro/fhir/data/resource_model/AllergyIntoleranceDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/AppointmentDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/ConditionDiagnosisDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/DiagnosticReportDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/EncounterDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/FamilyMemberHistoryDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/LabTestAndComponentModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java(1 hunks)src/main/java/com/wipro/fhir/data/resource_model/VitalsAnthropometryModel.java(1 hunks)src/main/java/com/wipro/fhir/data/users/User.java(2 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- src/main/java/com/wipro/fhir/data/resource_model/VitalsAnthropometryModel.java
- src/main/java/com/wipro/fhir/data/resource_model/FamilyMemberHistoryDataModel.java
- src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java
- src/main/java/com/wipro/fhir/data/resource_model/DiagnosticReportDataModel.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (11)
src/main/java/com/wipro/fhir/data/users/User.java (2)
5-5: Add Jackson import for JSON deserialization
ImportingJsonIgnorePropertiesenables the annotation to ignore unexpected fields in incoming payloads.
18-18: Ignore unknown JSON properties onUser
The@JsonIgnoreProperties(ignoreUnknown = true)annotation prevents failures when extra JSON fields are present, improving robustness.src/main/java/com/wipro/fhir/data/resource_model/FamilyMemberHistoryDataModel.java (3)
60-69: Excellent implementation of comprehensive null checks.The ternary operator approach provides consistent null handling across all fields, and the safer casting via
((Number) objArr[x]).longValue()improves type safety for BigInteger conversions.
60-69: Excellent consistent null checking implementation.This constructor demonstrates good practices:
- Consistent ternary operator approach for all fields
- Safer numeric casting using
Number.longValue()for BigInteger fields- Complete null checking coverage
60-69: Excellent implementation of null safety.This constructor demonstrates best practices with consistent null checks for all fields and safer type casting using
Number.longValue(). The ternary operator approach is clean and readable.src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java (3)
82-105: Well-implemented comprehensive null checking.The consistent use of ternary operators for null checks and the safer
((Number) objArr[x]).longValue()casting approach significantly improves robustness and type safety.
82-105: Consistent and safe null checking implementation.The constructor follows best practices with:
- Consistent ternary operator pattern for all assignments
- Safe numeric casting using
Number.longValue()- Complete null checking coverage for all assigned fields
82-104: Excellent comprehensive null safety implementation.The constructor properly handles all potential null values with consistent ternary operator patterns and uses safer
Number.longValue()casting for BigInteger fields. This implementation aligns well with the defensive programming approach.src/main/java/com/wipro/fhir/data/resource_model/DiagnosticReportDataModel.java (3)
70-87: Excellent comprehensive null checking implementation.The consistent ternary operator approach and safer BigInteger casting via
((Number) objArr[x]).longValue()significantly improves code robustness and type safety.
70-88: Excellent implementation of null checking pattern.The constructor demonstrates consistent best practices:
- Uniform ternary operator pattern for all field assignments
- Safe numeric casting using
Number.longValue()for BigInteger fields- Complete null checking coverage
- Appropriately leaves fields with default values unassigned
70-87: Excellent null safety implementation.The constructor demonstrates proper defensive programming with consistent null checks and safer type casting. The ternary operator approach is clean and maintains good code readability.
src/main/java/com/wipro/fhir/data/resource_model/VitalsAnthropometryModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/FamilyMemberHistoryDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/DiagnosticReportDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/AppointmentDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/LabTestAndComponentModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/EncounterDataModel.java
Show resolved
Hide resolved
src/main/java/com/wipro/fhir/data/resource_model/MedicationRequestDataModel.java
Show resolved
Hide resolved
|
2 participants
📋 Description
JIRA ID: AMM-1452
✅ 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