Skip to content

Reduce code duplication in converters and event emitters#1090

Merged
EdwardCooke merged 3 commits intoaaubry:masterfrom
fdcastel:refactor/code-dedup-simplification
Apr 9, 2026
Merged

Reduce code duplication in converters and event emitters#1090
EdwardCooke merged 3 commits intoaaubry:masterfrom
fdcastel:refactor/code-dedup-simplification

Conversation

@fdcastel
Copy link
Copy Markdown
Contributor

Summary

Reduce code duplication across DateTime/TimeOnly converters and event emitter scalar rendering.

Extract ScalarConverterBase<T> for converter family

Created new abstract base class ScalarConverterBase<T> providing:

  • Generic Accepts(Type) method — eliminates identical boilerplate from 5 converter classes
  • ConsumeScalarValue() helper for reading scalar values from the parser
  • EmitScalar() helper for writing scalars with a given style

Migrated 5 converters to inherit from the base:

  • DateTimeConverter
  • DateTimeOffsetConverter
  • DateTime8601Converter
  • DateOnlyConverter
  • TimeOnlyConverter

Align event emitter scalar rendering

  • JsonEventEmitter: Use formatter.FormatNumber() for float, double, and decimal instead of direct CultureInfo.InvariantCulture formatting. This aligns with TypeAssigningEventEmitter and ensures the configured formatter is respected. NaN/Infinity double-quoting is preserved.
  • TypeAssigningEventEmitter: Use .IsEnum() extension method instead of .IsEnum property for enum detection, matching the JsonEventEmitter pattern.
  • Removed unused System.Globalization and System.Text.RegularExpressions imports from JsonEventEmitter.

Files Changed

  • YamlDotNet/Serialization/Converters/ScalarConverterBase.cs (new)
  • YamlDotNet/Serialization/Converters/DateTimeConverter.cs
  • YamlDotNet/Serialization/Converters/DateTimeOffsetConverter.cs
  • YamlDotNet/Serialization/Converters/DateTime8601Converter.cs
  • YamlDotNet/Serialization/Converters/DateOnlyConverter.cs
  • YamlDotNet/Serialization/Converters/TimeOnlyConverter.cs
  • YamlDotNet/Serialization/EventEmitters/JsonEventEmitter.cs
  • YamlDotNet/Serialization/EventEmitters/TypeAssigningEventEmitter.cs

Validation

All 1845 tests pass (4 pre-existing locale-dependent float formatting failures unrelated to these changes).

Create abstract ScalarConverterBase<T> providing shared functionality:
- Generic Accepts(Type) method (eliminates boilerplate from 5 classes)
- ConsumeScalarValue() helper for reading scalars
- EmitScalar() helper for writing scalars with a given style

Migrate DateTimeConverter, DateTimeOffsetConverter, DateTime8601Converter,
DateOnlyConverter, and TimeOnlyConverter to inherit from the new base class.
- JsonEventEmitter: use formatter.FormatNumber() for float/double/decimal
  instead of direct CultureInfo.InvariantCulture formatting, matching the
  pattern used by TypeAssigningEventEmitter. Preserves NaN/Infinity quoting.
- TypeAssigningEventEmitter: use .IsEnum() extension method instead of
  .IsEnum property for enum detection, matching JsonEventEmitter pattern.
- Remove unused System.Globalization and System.Text.RegularExpressions
  imports from JsonEventEmitter.
The FormatNumber() method uses YAML-style symbols (.nan, .inf, -.inf)
which are incorrect for JSON output. For NaN/Infinity values, use
CultureInfo.InvariantCulture to produce standard representations
(NaN, Infinity, -Infinity) while still using FormatNumber() for
regular numeric values.
@fdcastel
Copy link
Copy Markdown
Contributor Author

Fixed the CI failure in commit 07c17d4.

Root cause: The refactoring of JsonEventEmitter to use formatter.FormatNumber() instead of direct CultureInfo.InvariantCulture formatting inadvertently changed how NaN and Infinity values are rendered. The YamlFormatter.FormatNumber() uses YAML-style symbols (.nan, .inf, -.inf), but JSON output needs the standard .NET representations (NaN, Infinity, -Infinity).

Fix: For float/double values that are NaN or Infinity, use CultureInfo.InvariantCulture formatting (preserving original behavior). For regular numeric values, continue using formatter.FormatNumber() (the deduplication goal of this PR).

The SerializationOfNumericsAsJsonRountTrip test now passes on all frameworks.

@EdwardCooke EdwardCooke merged commit 3899001 into aaubry:master Apr 9, 2026
1 check passed
This was referenced Apr 9, 2026
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