Skip to content

ZmanimFormatter.setTimeFormat: leave the formatter unchanged when the value is invalid - #303

Merged
KosherJava merged 1 commit into
KosherJava:masterfrom
Elyahu41:fix/set-time-format-validation
Nov 6, 2026
Merged

KosherJava merged 1 commit into
KosherJava:masterfrom
Elyahu41:fix/set-time-format-validation

Conversation

@Elyahu41

@Elyahu41 Elyahu41 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

The issue

ZmanimFormatter.setTimeFormat(int) throws IllegalArgumentException for an invalid value, but only after it has already changed the formatter. That leaves the formatter in a state that matches neither the old format nor the new one.

The edge case

Calling setTimeFormat with a value that isn't one of the five constants, such as a format read from preferences or a config file. Callers can reasonably expect to catch the exception and keep the previous format.

What happens

this.timeFormat = format; runs before the switch validates format. The switch then throws, but timeFormat now holds the invalid value. If the formatter was using XSD_DURATION_FORMAT, format(Duration) stops using it and falls back to whatever sexagesimal settings were last applied:

ZmanimFormatter formatter = new ZmanimFormatter(ZmanimFormatter.XSD_DURATION_FORMAT, DateTimeFormatter.ofPattern("h:mm:ss"), zone);
formatter.format(Duration.ofMinutes(90)); // "PT1H30M"
try { formatter.setTimeFormat(5); } catch (IllegalArgumentException e) { /* keep the old format */ }
formatter.format(Duration.ofMinutes(90)); // "1:30"   (expected "PT1H30M")

The fix

Validate first: assign timeFormat after the switch, once the value is known to be valid. An invalid value now throws and leaves the formatter unchanged, which the Javadoc now states. Valid values behave exactly as before.

Tests

ZmanimFormatterTimeFormatTest:

  • XSD format: After an invalid value, an XSD-format formatter still formats PT1H30M. This fails without the fix.
  • Sexagesimal format: After an invalid value, a sexagesimal formatter still formats 1:30, and a valid change afterwards works.

🤖 Generated with Claude Code

https://claude.ai/code/session_015Er4Sx7LnKzzCbgDEatxkU

… is invalid

timeFormat was assigned before the value was validated, so an invalid value
threw IllegalArgumentException but left the formatter in a mixed state: a
formatter using XSD_DURATION_FORMAT went on to format durations as "1:30"
with the settings of the last sexagesimal format instead of "PT1H30M".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Er4Sx7LnKzzCbgDEatxkU
@KosherJava
KosherJava merged commit c73b628 into KosherJava:master Nov 6, 2026
3 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