Skip to content

refactor: replace asserts with explicit exceptions - #1312

Merged
bengbengbalabalabeng merged 1 commit into
apache:mainfrom
bengbengbalabalabeng:refactor-replace-assert
Nov 6, 2026
Merged

bengbengbalabalabeng merged 1 commit into
apache:mainfrom
bengbengbalabalabeng:refactor-replace-assert

Conversation

@bengbengbalabalabeng

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Replacing them with explicit exceptions ensures fail-fast behavior and provides clearer failure messages.

What's changed?

  • Replaced assert with IllegalArgumentException and ExcelAnalysisException in CsvSheet, CsvWorkbook, and BofRecordHandler.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.
    • No new unit test added for BofRecordHandler: this is a purely defensive guard against malformed/corrupted .xls files (e.g. missing BoundSheetRecord), not a functional regression.

@nkuprins nkuprins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@GOODBOY008 GOODBOY008 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@bengbengbalabalabeng
bengbengbalabalabeng merged commit ac4d474 into apache:main Nov 6, 2026
9 checks passed
@bengbengbalabalabeng
bengbengbalabalabeng deleted the refactor-replace-assert branch November 6, 2026 11:14
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.

3 participants