chan-dx commented on code in PR #4229:
URL: https://github.com/apache/solr/pull/4229#discussion_r2974673760
##########
solr/core/src/java/org/apache/solr/schema/numericrange/AbstractNumericRangeField.java:
##########
@@ -82,9 +83,54 @@ public interface NumericRangeValue {
protected static final Pattern SINGLE_BOUND_PATTERN =
Pattern.compile("^" + COMMA_DELIMITED_NUMS + "$");
+ /**
+ * Regex fragment matching a comma-separated list of signed floating-point
numbers (integers or
+ * floating-point literals).
+ */
+ protected static final String COMMA_DELIMITED_FP_NUMS =
Review Comment:
Your current approach is already really clean IMO, since it keeps validation
in one place.
I did consider a more type agnostic two-step approach: use just one regex to
validate the overall` [... TO ...] `structure to reduce some of the regex
complexity, then let `parseFloatArray` in `FloatRangeField.java` reject invalid
values downstream. But mismatched-dimension edge cases would need separate
handling, so that ends up splitting the validation rather than really
simplifying it, which also doesn't align with your rationale.
- One small cleanup idea: since `RANGE_PATTERN_STR` and
`FP_RANGE_PATTERN_STR` share the same overall structure and mainly differ in
the numeric regex fragment they embedded, would it be worth introducing a small
helper like `buildRangePattern(String numericFragment)` to centralise the range
pattern assembly, and maybe even the compilation step as well? That might
reduce the duplication a little.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
