Skip to content

Validate Roman numeral parser input - #112

Closed
krotname wants to merge 1 commit into
mainfrom
codex/fix-roman-numeral-converter-input-validation
Closed

Validate Roman numeral parser input#112
krotname wants to merge 1 commit into
mainfrom
codex/fix-roman-numeral-converter-input-validation

Conversation

@krotname

@krotname krotname commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The fromRoman implementation unconditionally used new StringBuilder(romanNumeral) and unboxed MAP_FROM.get(c), which caused NullPointerExceptions for null or unsupported input, and it accumulated into an int allowing silent overflow for very large inputs.

Description

  • Added a null check to fromRoman that throws IllegalArgumentException for null input.
  • Added validation for unsupported characters by checking MAP_FROM.get(c) and throwing IllegalArgumentException when a character is not recognized.
  • Changed the accumulator from int to long and returned Math.toIntExact(result) so values outside the int range trigger an ArithmeticException instead of wrapping.
  • Added regression tests in src/test/java/kyu4/RomanNumeralsTest.java that assert proper exceptions for null, invalid characters, malformed input, and the integer-overflow boundary while keeping existing round-trip and conversion tests.

Testing

  • Ran mvn -q -Dtest=kyu4.RomanNumeralsTest test and the targeted test class passed.
  • Ran mvn test and the full test suite completed successfully with 601 tests passing and no failures.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1894bceebc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/main/java/kyu4/RomanNumerals.java
@krotname

krotname commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Закрываю как superseded by #116: изменения перенесены в одну проверенную ветку, все review findings учтены; локальный mvn verify прошёл полностью.

@krotname krotname closed this Aug 1, 2026
@krotname
krotname deleted the codex/fix-roman-numeral-converter-input-validation branch August 1, 2026 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant