Copilot commented on code in PR #2669:
URL: https://github.com/apache/sedona/pull/2669#discussion_r2837171780
##########
common/src/main/java/org/apache/sedona/common/raster/cog/CogOptions.java:
##########
@@ -206,7 +205,9 @@ public CogOptions build() {
if (compression == null || compression.isEmpty()) {
throw new IllegalArgumentException("compression must not be null or
empty");
}
- if (!VALID_COMPRESSION.contains(compression)) {
+ // Case-insensitive matching: find the canonical value from the valid
list
+ this.compression = matchIgnoreCase(VALID_COMPRESSION, compression);
+ if (this.compression == null) {
throw new IllegalArgumentException(
"compression must be one of " + VALID_COMPRESSION + ", got: '" +
compression + "'");
}
Review Comment:
In `build()`, `this.compression` is overwritten with the matched canonical
value before validating it’s non-null. If the input is invalid, the thrown
message ends up saying `got: 'null'` (because the field has already been set to
null), which makes the error misleading. Keep the original input in a local
variable and only assign `this.compression` after a successful match so the
exception reports the actual value provided.
```suggestion
// Preserve the original input for error reporting.
String originalCompression = compression;
// Case-insensitive matching: find the canonical value from the valid
list
String normalizedCompression = matchIgnoreCase(VALID_COMPRESSION,
originalCompression);
if (normalizedCompression == null) {
throw new IllegalArgumentException(
"compression must be one of " + VALID_COMPRESSION + ", got: '" +
originalCompression + "'");
}
this.compression = normalizedCompression;
```
--
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]
