Skip to content

Review/en#2

Open
enijburg wants to merge 6 commits intomasterfrom
review/en
Open

Review/en#2
enijburg wants to merge 6 commits intomasterfrom
review/en

Conversation

@enijburg
Copy link
Copy Markdown
Collaborator

@enijburg enijburg commented Apr 10, 2026

Code review and suggested changes

This pull request introduces several improvements and refactorings to the SharpFunctional.MSSQL codebase, focusing on enhanced dependency injection, improved time abstraction for testing, better exception handling, and more explicit transaction management. The changes also clarify and enforce constructor parameters, update logging extension methods, and ensure consistent naming across the API.

Dependency Injection and API Consistency

  • Refactored constructors and service registration to consistently use the parameter name dbConnection instead of connection, improving clarity and aligning with usage in both FunctionalMsSqlDb and dependency injection extension methods. [1] [2] [3] [4] [5] [6]
  • Updated the FunctionalMsSqlDb class to enforce that either a DbContext or a dbConnection must be provided, throwing an exception otherwise, and added a runtime check in InTransactionAsync. [1] [2]
  • Changed all usages of AmbientTransaction to use the new GetAmbientTransaction() method for better encapsulation. [1] [2] [3] [4] [5] [6]

Circuit Breaker Improvements

  • Refactored the CircuitBreaker to accept an optional TimeProvider, replacing direct calls to DateTime.UtcNow with TimeProvider.System by default. This makes the circuit breaker more testable and flexible. [1] [2] [3] [4] [5]

Exception Handling and Functional Extensions

  • Improved exception handling in functional extension methods: now only OperationCanceledException is rethrown, while other exceptions are caught and result in an empty Option or Seq, with a TODO comment suggesting future logging or richer error handling. [1] [2] [3]

Logging API Modernization

  • Updated logging extension methods in FunctionalMsSqlDbLog to use this ILogger logger for improved extension method syntax and consistency with .NET logging best practices.

Minor Fixes and Cleanups

  • Updated the Dapper example code comment for consistency with constructor parameter changes.
  • Minor refactor to prefer direct use of dbContext in place of the old EfContext property.

These changes collectively improve the maintainability, testability, and clarity of the codebase.

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.

1 participant