Skip to content

Conversation

@jbrinkman
Copy link
Owner

📋 Overview

This PR refactors the CompareCommand class to use proper dependency injection patterns, eliminating the service locator anti-pattern and improving code quality.

🎯 Problem Solved

The original issue was that namespace mappings from JSON configuration files were not being applied. Investigation revealed that the root cause was improper dependency injection patterns where services were being resolved repeatedly through GetRequiredService calls.

✅ Changes Made

Core Refactoring

  • CompareCommand Constructor: Changed from 2 parameters → 4 parameters
    • Added IExitCodeManager exitCodeManager parameter
    • Added IGlobalExceptionHandler exceptionHandler parameter
    • Eliminated repeated GetRequiredService calls in favor of constructor injection

Supporting Changes

  • TypeRegistrar: Updated to provide the new constructor parameters
  • Program.cs: Made ConfigureServices method public for testing accessibility
  • DI Validation: Added comprehensive DependencyInjectionTests.cs with 13 test methods

Test Updates

  • CompareCommandTests.cs: Updated to use existing CreateCompareCommand() helper
  • CompareCommandErrorTests.cs: Updated with existing mock objects
  • CompareCommandFilteringTests.cs: Added new helper method for dependency resolution

🧪 Test Results

  • Total Tests: 363
  • Passed: 363 ✅
  • Failed: 0 ✅
  • Skipped: 0 ✅

All existing functionality is preserved and fully tested.

🏗️ Architecture Improvements

  • Better Performance: Constructor injection is more efficient than repeated service resolution
  • Improved Testability: Dependencies are now explicit and easier to mock
  • Enhanced Maintainability: Clear dependency declarations make the code more maintainable
  • Proper DI Patterns: Eliminated service locator anti-pattern

📂 Files Modified

  • src/DotNetApiDiff/Commands/CompareCommand.cs
  • src/DotNetApiDiff/Commands/TypeRegistrar.cs
  • src/DotNetApiDiff/Program.cs
  • tests/DotNetApiDiff.Tests/Integration/DependencyInjectionTests.cs (new)
  • tests/DotNetApiDiff.Tests/Commands/CompareCommandTests.cs
  • tests/DotNetApiDiff.Tests/Commands/CompareCommandErrorTests.cs
  • tests/DotNetApiDiff.Tests/Commands/CompareCommandFilteringTests.cs

🔄 Next Steps

With the DI architecture properly implemented, the next phase will be to investigate the original namespace mapping configuration issue to ensure JSON configuration mappings are correctly applied to the API comparison logic.

✅ Ready to Merge

  • All tests passing
  • Code builds successfully
  • No breaking changes to public API
  • Proper dependency injection patterns implemented
  • Comprehensive test coverage

Related to issue #28

- Change CompareCommand constructor from 2 to 4 parameters
- Add IExitCodeManager and IGlobalExceptionHandler parameters
- Eliminate service locator anti-pattern by using constructor injection
- Update TypeRegistrar to provide new constructor parameters
- Make Program.ConfigureServices public for testing
- Add comprehensive DI validation tests (DependencyInjectionTests.cs)
- Fix all unit tests to use new constructor parameters
- All 363 tests now pass

This improves performance, testability, and maintainability by using proper DI patterns.
Updated CliWorkflowTests to properly validate error messages from new
logging-based exception handling. Tests now check for formatted error
messages that include the actual filename in the output.
@jbrinkman jbrinkman merged commit 40ecbc9 into main Jul 28, 2025
7 checks passed
@jbrinkman jbrinkman deleted the feature/refactor-comparecommand-di branch July 28, 2025 16:55
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