Skip to content

Java API interface updated to give results from implementation classes.#630

Merged
sparkhi merged 2 commits intomasterfrom
update-result-interface-to-publicise-errors
Jan 13, 2026
Merged

Java API interface updated to give results from implementation classes.#630
sparkhi merged 2 commits intomasterfrom
update-result-interface-to-publicise-errors

Conversation

@sparkhi
Copy link
Contributor

@sparkhi sparkhi commented Jan 12, 2026

Java API had a Result interface which needs to return errors from the corresponding implementation classes instead of an empty ArrayList defined in the interface.

  • The interface updated to add a method.
  • Implementation classes implement that method
  • Removed the Request interface as it is not needed (all setups happen through the builder pattern)
  • Corresponding case classes updated to not extend from Request
  • Tests updated with minor name change

Copy link
Contributor

@techncl techncl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just some minor comments.

Assert.assertFalse(validatorRequest.validatorRequest().skipFileChecks());
Assert.assertEquals(4096, validatorRequest.validatorRequest().maxCharsPerCellLimit());
CsvValidatorJavaBridge.ValidationResult validationResult = (CsvValidatorJavaBridge.ValidationResult) result;
Assert.assertEquals("csvFile", validationResult.validatorRequest().csvFile());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, do we need to do validationResult.validatorRequest() each time? Couldn't we save it to a variable? same question for each test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. I just renamed the 'Result' variable which was called 'Request' earlier

Copy link
Contributor

@techncl techncl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sparkhi sparkhi merged commit df58e18 into master Jan 13, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants