Skip to content

Conversation

aarondglover
Copy link
Contributor

Hi @davebronson

Apologies for this mega PR! I tangled up my Git commits and merged back into master too early — so this ended up as two PRs in one.

Feature 1 – DateTime Precision Configuration

This feature introduces granular control over how DateTime values are formatted in HL7 messages. You can:

  1. Define a global DateTime precision for all fields.
  2. Override specific fields with type-safe per-field configuration.
  3. Rely on full backward compatibility — if no precision config is defined, nothing breaks.

Feature 2 – EnumHelper as Static

EnumHelper can now be used as a static utility.

  • Instantiating it felt awkward since the methods are pure functions with no state.
  • Making them static makes the API more idiomatic with other C# libraries.
  • There’s a slight performance improvement with static methods.

Compatibility Note:

  • The class still implements IEnumHelper, and the instance methods simply forward to the static ones.
  • This avoids breaking changes for existing DI setups or code using new EnumHelper().
  • New usage will naturally lean toward the static methods.

Copilot AI and others added 7 commits September 24, 2025 02:16
…hods

* Initial plan

* Convert all EnumHelper classes to static classes

* Add static EnumCodes class for backward-compatible static access

* Implement static methods with explicit interface implementation to eliminate duplication
…hods

* Initial plan

* Convert all EnumHelper classes to static classes

* Add static EnumCodes class for backward-compatible static access

* Implement static methods with explicit interface implementation to eliminate duplication
…ing with original precision preservation (#3)

This pull request introduces a comprehensive and flexible DateTime precision configuration system for HL7 message formatting, enabling both global and type-safe per-field control over DateTime serialization. The changes are fully backward compatible and include extensive documentation, new extension methods, and an example usage file.

**Key changes:**

### DateTime Precision Configuration System

* Introduced a new `DateTimePrecision` enum in `Enumerations.cs` to represent precision levels (Year, Month, Day, Hour, Minute, Second).
* Added new extension methods in `StringExtensions.cs` for formatting `DateTime` and nullable `DateTime` values with explicit precision or per-field configuration, including logic to resolve the correct format based on configuration hierarchy.

### Documentation and Examples

* Added a detailed documentation file `DateTime-Precision-Configuration.md` describing the configuration API, usage scenarios, available precision formats, and hierarchy of overrides.
* Updated the main `README.md` with a new section summarizing DateTime precision configuration, its features, usage, and a reference to the detailed documentation. [[1]](diffhunk://#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R284-R324) [[2]](diffhunk://#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R611)
* Added a new example file `examples/DateTimePrecisionExample.cs` demonstrating the configuration and usage of the new DateTime precision features, including both global and per-field overrides.
…convert ALL segments (#5) (#6)

Fix DateTime precision hierarchy, add culture parameter support, and convert ALL segments (#5) (#6)

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: aarondglover <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 20:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@davebronson
Copy link
Owner

Hi @aarondglover , thank you for this work! Please bear with me while I spend a bit of time reviewing the changes. I'll probably get to it over the coming weekend.

Copy link
Owner

@davebronson davebronson left a comment

Choose a reason for hiding this comment

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

Hi Aaron. A few items in the PR to look at, if you would.

Copilot AI and others added 6 commits October 1, 2025 01:06
* Add null-conditional operator to all ToHl7DateTimeString calls

Co-authored-by: aarondglover <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: aarondglover <[email protected]>
…ension method (#8)

I had a problem - so I tried to use regex. Then I had 264 problems! Missed some files particularly those which didn't define a culture and/or were a TimeOnly type of constant


* Update ToDelimitedString methods in Type classes to use ToHl7DateTimeString

Co-authored-by: aarondglover <[email protected]>

* Update Date and Time format ToString patterns to use ToHl7DateTimeString

Co-authored-by: aarondglover <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: aarondglover <[email protected]>
…te namespace (#9)

* Move DateTimePrecisionExample.cs to ClearHl7/Examples folder and update namespace and add missing arguments.

Co-authored-by: aarondglover <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: aarondglover <[email protected]>
* Initial plan

* Remove duplicate XML documentation tags from all EnumHelper classes

Co-authored-by: aarondglover <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: aarondglover <[email protected]>
…thods (#11)

Reverted all static methods to public instance methods and removed the redundant explicit interface implementations:

Co-authored-by: aarondglover <[email protected]>

* Update class documentation to remove outdated static method reference

Co-authored-by: aarondglover <[email protected]>

* Add comprehensive unit tests for EnumHelper instance methods

Co-authored-by: aarondglover <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: aarondglover <[email protected]>
* Convert EnumHelper methods from static to instance to fix breaking change
* Update class documentation to remove outdated static method reference
* Add comprehensive unit tests for EnumHelper instance methods
* Add static EnumHelpers facade and revert XML documentation in EnumHelper classes
* Add comprehensive tests for static EnumHelpers facade
* Update README.md to document static EnumHelpers usage
* Update class documentation to remove outdated static method reference
@davebronson
Copy link
Owner

Hi @aarondglover, this is looking good. Thank you for your efforts! Do you think having both /Helpers/EnumHelper and /Helpers/EnumHelpers classes (similarly named) would be confusing? I feel that it might be. Should we keep it as-is, or choose a different class name? What do you think of EnumService, or other?

If it's changed to EnumService, that would mean it'd have to live in a Service folder and namespace.

You don't have to make any changes at the moment. I'm just looking for thoughts on the naming.

@aarondglover
Copy link
Contributor Author

@davebronson ... the “Helper vs Helpers” pair is a little bit irksome.

Constraints: As you know I can’t rename the existing EnumHelper (that would be a breaking change). The other constraint and original reason for doing this was to be idiomatic and sympathetic to established C# norms.

Why not EnumService?

  • Misleading semantics: “Service” implies orchestration, I/O, or state. This is just pure, in-memory mapping.
  • DI expectations: The name suggests lifetimes, config, retries, etc.—none apply here.
  • Least surprise: Callers expect latency/failure from a “service”; this is deterministic and side-effect-free.
  • Architecture fit: It’s a utility concern, not an application/service layer concern.

Better names: EnumUtility, EnumHelper, or EnumCodeMap (static facade) while keeping EnumHelper for DI.

Proposal:

  • Keep the existing DI/instance type: IEnumHelper + EnumHelper (no change).

  • Rename the new static facade from EnumHelpers to something less confusable, e.g. EnumUtility (or EnumCodes / EnumCodeMap).

    • Purpose is purely static, stateless convenience; “Service” suggests state/workflow, which this isn’t.
    • Lives alongside /Helpers with XML docs clearly pointing DI users to IEnumHelper and static callers to EnumUtility.

This avoids a breaking rename, removes the look-alike names, and keeps intent clear:

  • DI: IEnumHelper/EnumHelper
  • Static: EnumUtility.EnumToCode(...)

If you prefer a specific name (EnumUtility vs EnumCodes), I’ll align.

@davebronson
Copy link
Owner

@aarondglover Either EnumUtility or EnumMaps would be a good alternative, I think, yes. I like the later, but pease go ahead and pick one (or a similar alternative) and run with it.

An additional thought I had is the choice to have the new static enum methods point at the old EnumHelper methods for their return values. If the decision to provide static methods was to make the API more idiomatic with other libraries, and to also provide a small performance improvement, I think the mapping logic should live in the new static classes. And have the old EnumHelper methods be updated to point at the new static methods for its return values. So basically the reverse of what's currently in this PR. Doing this would make the static methods slightly more performant and also make it easier to deprecate EnumHelper in the future without having to change any code. Thoughts?

@davebronson
Copy link
Owner

An example, for clarity:

namespace ClearHl7.Codes.V282.Helpers
{
    public static class EnumMaps
    {
		/// <summary>
		/// Converts the given CodeAcceptApplicationAcknowledgmentConditions enum value into its HL7 equivalent code.
		/// </summary>
		/// <param name="input">An enum value to convert.</param>
		/// <returns>A string.</returns>
        public static string EnumToCode(CodeAcceptApplicationAcknowledgmentConditions input)
        {
			return input switch
			{
				CodeAcceptApplicationAcknowledgmentConditions.Always => "AL",
				CodeAcceptApplicationAcknowledgmentConditions.ErrorRejectConditionsOnly => "ER",
				CodeAcceptApplicationAcknowledgmentConditions.Never => "NE",
				CodeAcceptApplicationAcknowledgmentConditions.SuccessfulCompletionOnly => "SU",
				_ => throw new NotImplementedException()
			};
		}
		
		// [...]
}


namespace ClearHl7.Codes.V282.Helpers
{
    public class EnumHelper : IEnumHelper
    {
        /// <inheritdoc />
        public string EnumToCode(CodeAcceptApplicationAcknowledgmentConditions input)
			=> EnumMaps.EnumToCode(input);

		// [...]
	}
}

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