Conversation
WalkthroughThis change modifies the FLOAT value formatting in the PDF form rendering by introducing BigDecimal conversion from string representation to improve numeric precision before applying the number format. Null handling and color logic remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
e54b9c1 to
eaa0b87
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/main/java/org/phoenixctms/ctsms/pdf/InputFieldPDFBlock.java (1)
3-3: BigDecimal-based formatting aligns with goal; consider guarding NaN/Infinity for robustnessUsing
BigDecimalconstructed from theFloat’s string representation is a reasonable way to stabilize decimal formatting and matches the PR’s intent of reducing precision artefacts. The import at Line 3 and the updated FLOAT branch at Lines 485–497 look consistent with the surrounding formatting logic and keep null/preset handling and coloring intact.One thing to double‑check:
Float.toStringreturns"NaN","Infinity", or"-Infinity"for non‑finite values.new BigDecimal("NaN")(or"Infinity","-Infinity") will throwNumberFormatException, which would now break PDF rendering if such a value ever slipped through validation. Previously,DecimalFormat.format(Float.NaN)would not throw.If it is possible (even theoretically) for
getFloatValue(...)to return NaN or Infinity, consider a small guard, e.g.:- value = getFloatValue(blank); - numberFormat = getFloatValueFormat(); - if (value != null) { - string = numberFormat.format(new BigDecimal(value.toString())); + Float floatValue = getFloatValue(blank); + numberFormat = getFloatValueFormat(); + if (floatValue != null) { + if (floatValue.isNaN() || floatValue.isInfinite()) { + // Fall back to the plain string representation or previous behavior + string = floatValue.toString(); + } else { + string = numberFormat.format(new BigDecimal(floatValue.toString())); + } } else { string = (isShowPresetValues() ? getFloatValueFormatPattern() : null); isPresetColor = true; }If your validation guarantees only finite values for FLOAT fields, the current implementation is fine but it would be good to document that invariant near
getFloatValue/ the field definition.Also applies to: 485-497
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/src/main/java/org/phoenixctms/ctsms/pdf/InputFieldPDFBlock.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.