Abacn commented on code in PR #31869:
URL: https://github.com/apache/beam/pull/31869#discussion_r1676476609
##########
sdks/java/io/csv/src/main/java/org/apache/beam/sdk/io/csv/CsvIOParseHelpers.java:
##########
@@ -66,8 +67,20 @@ static void validate(CSVFormat format) {
* Validate the {@link CSVFormat} in relation to the {@link Schema} for CSV
record parsing
* requirements.
*/
- // TODO(https://github.com/apache/beam/issues/31716): implement method.
- static void validate(CSVFormat format, Schema schema) {}
+ static void validate(CSVFormat format, Schema schema) {
Review Comment:
This is an overload method. For overload method we usually want to have code
path for logics, e.g.
```
static void validate(CSVFormat format) {
validate(format, null);
}
validate(CSVFormat format, @Nullable Schema schema) {
// validate format alone
...
if (schema != null) {
// validate format against schema
...
}
}
```
if it serves as different purpose as the other validate, we should use
different naming (e.g. validateCsvFormat, validateCsvFormatWithSchema, etc)
instead of using same name
##########
sdks/java/io/csv/src/main/java/org/apache/beam/sdk/io/csv/CsvIOParseHelpers.java:
##########
@@ -66,8 +67,20 @@ static void validate(CSVFormat format) {
* Validate the {@link CSVFormat} in relation to the {@link Schema} for CSV
record parsing
* requirements.
*/
- // TODO(https://github.com/apache/beam/issues/31716): implement method.
- static void validate(CSVFormat format, Schema schema) {}
+ static void validate(CSVFormat format, Schema schema) {
Review Comment:
This is an overload method. For overload method we usually want to have
single code path for logics, e.g.
```
static void validate(CSVFormat format) {
validate(format, null);
}
validate(CSVFormat format, @Nullable Schema schema) {
// validate format alone
...
if (schema != null) {
// validate format against schema
...
}
}
```
if it serves as different purpose as the other validate, we should use
different naming (e.g. validateCsvFormat, validateCsvFormatWithSchema, etc)
instead of using same name
--
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]
