Skip to content

Commit 0d35bdc

Browse files
floriankraemerfloriankraemer
committed
Refactoring
1 parent 51d23a5 commit 0d35bdc

30 files changed

+1514
-134
lines changed

‎docs/SpecificationPattern-Implementation.md‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
# Specification Pattern Implementation for ChurnCommand
2+
3+
## Overview
4+
5+
The Specification pattern has been successfully implemented in the `ChurnCommand` to reduce conditional complexity and improve maintainability. This refactoring separates validation logic from business logic and makes the code more testable and extensible.
6+
7+
## Files Created
8+
9+
### Core Pattern Files
10+
- `src/Command/ChurnSpecifications/ChurnCommandContext.php` - Context object holding command input data
11+
- `src/Command/ChurnSpecifications/ChurnCommandValidationSpecification.php` - Base interface for specifications
12+
- `src/Command/ChurnSpecifications/CompositeChurnValidationSpecification.php` - Composite pattern for combining specifications
13+
14+
### Individual Specifications
15+
- `src/Command/ChurnSpecifications/CoverageFormatExclusivitySpecification.php` - Validates only one coverage format is specified
16+
- `src/Command/ChurnSpecifications/CoverageFileExistsSpecification.php` - Validates coverage file exists
17+
- `src/Command/ChurnSpecifications/CoverageFormatSupportedSpecification.php` - Validates coverage format is supported
18+
- `src/Command/ChurnSpecifications/ReportOptionsCompleteSpecification.php` - Validates report options are complete
19+
20+
### Test File
21+
- `tests/Command/ChurnSpecifications/ChurnSpecificationPatternTest.php` - Unit tests demonstrating the pattern
22+
23+
## Key Changes
24+
25+
### Before (Original ChurnCommand.execute method)
26+
```php
27+
protected function execute(InputInterface $input, OutputInterface $output): int
28+
{
29+
// Load configuration if provided
30+
$configFile = $input->getOption(self::OPTION_CONFIG_FILE);
31+
if ($configFile !== null) {
32+
if (!$this->loadConfiguration($configFile, $output)) {
33+
return self::FAILURE;
34+
}
35+
}
36+
37+
$coberturaFile = $input->getOption(self::OPTION_COVERAGE_COBERTURA);
38+
$cloverFile = $input->getOption(self::OPTION_COVERAGE_CLOVER);
39+
40+
// Validate that only one coverage option is specified
41+
if ($coberturaFile !== null && $cloverFile !== null) {
42+
$output->writeln('<error>Only one coverage format can be specified at a time.</error>');
43+
return self::FAILURE;
44+
}
45+
46+
$coverageFile = $coberturaFile ?? $cloverFile;
47+
$coverageFormat = $coberturaFile !== null ? 'cobertura' : ($cloverFile !== null ? 'clover' : null);
48+
49+
if (!$this->coverageFileExists($coverageFile, $output)) {
50+
return self::FAILURE;
51+
}
52+
53+
$coverageReader = $this->loadCoverageReader($coverageFile, $coverageFormat, $output);
54+
if ($coverageReader === false) {
55+
return self::FAILURE;
56+
}
57+
58+
// ... rest of method
59+
}
60+
```
61+
62+
### After (Refactored with Specifications)
63+
```php
64+
protected function execute(InputInterface $input, OutputInterface $output): int
65+
{
66+
$context = new CommandContext($input);
67+
68+
// Validate all specifications
69+
if (!$this->validationSpecification->isSatisfiedBy($context)) {
70+
$failedSpec = $this->validationSpecification->getFirstFailedSpecification($context);
71+
$output->writeln('<error>' . $failedSpec->getErrorMessage() . '</error>');
72+
return self::FAILURE;
73+
}
74+
75+
// Load configuration if provided
76+
if ($context->hasConfigFile()) {
77+
if (!$this->loadConfiguration($context->getConfigFile(), $output)) {
78+
return self::FAILURE;
79+
}
80+
}
81+
82+
// Load coverage reader
83+
$coverageReader = $this->loadCoverageReader($context, $output);
84+
if ($coverageReader === false) {
85+
return self::FAILURE;
86+
}
87+
88+
// ... rest of method
89+
}
90+
```
91+
92+
## Benefits Achieved
93+
94+
### 1. **Reduced Conditional Complexity**
95+
- Eliminated multiple nested if statements
96+
- Single validation point with clear error handling
97+
- Cleaner, more readable execute method
98+
99+
### 2. **Separation of Concerns**
100+
- Validation logic separated from business logic
101+
- Each validation rule is isolated and focused
102+
- Easy to understand what each specification validates
103+
104+
### 3. **Improved Testability**
105+
- Each specification can be unit tested independently
106+
- Mock CommandContext for testing different scenarios
107+
- Clear test cases for each validation rule
108+
109+
### 4. **Enhanced Maintainability**
110+
- Adding new validation rules requires only creating a new specification
111+
- No need to modify existing code
112+
- Easy to reorder or remove validation rules
113+
114+
### 5. **Better Reusability**
115+
- Specifications can be reused across different commands
116+
- Composite specifications can be combined in different ways
117+
- Easy to create command-specific validation sets
118+
119+
### 6. **Consistent Error Handling**
120+
- Standardized error message format
121+
- First failure stops validation chain (fail-fast)
122+
- Clear, specific error messages for each validation failure
123+
124+
## Usage Example
125+
126+
```php
127+
// Adding a new validation rule is now trivial
128+
class ConfigFileExistsSpecification implements CommandValidationSpecification
129+
{
130+
public function isSatisfiedBy(CommandContext $context): bool
131+
{
132+
$configFile = $context->getConfigFile();
133+
return $configFile === null || file_exists($configFile);
134+
}
135+
136+
public function getErrorMessage(): string
137+
{
138+
return sprintf('Configuration file not found: %s', $context->getConfigFile());
139+
}
140+
}
141+
142+
// Just add it to the composite specification
143+
$this->validationSpecification = new CompositeValidationSpecification([
144+
new CoverageFormatExclusivitySpecification(),
145+
new CoverageFileExistsSpecification(),
146+
new CoverageFormatSupportedSpecification(),
147+
new ReportOptionsCompleteSpecification(),
148+
new ConfigFileExistsSpecification(), // <- New validation rule
149+
]);
150+
```
151+
152+
## Testing
153+
154+
The implementation includes comprehensive unit tests demonstrating:
155+
- Individual specification validation
156+
- Composite specification behavior
157+
- Error message generation
158+
- Context object functionality
159+
160+
Run tests with:
161+
```bash
162+
phpunit tests/Command/ChurnSpecifications/ChurnSpecificationPatternTest.php
163+
```
164+
165+
## Conclusion
166+
167+
The Specification pattern implementation successfully reduces conditional complexity while improving code maintainability, testability, and extensibility. The refactored code is cleaner, more focused, and easier to extend with new validation rules.

‎src/Business/Churn/Report/ChurnReportFactory.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,13 @@ public function create(string $type): ReportGeneratorInterface
6363
*/
6464
private function createCustomExporter(array $config): ReportGeneratorInterface
6565
{
66+
$cognitiveConfig = $this->configService->getConfig();
67+
6668
$this->registry->loadExporter($config['class'], $config['file'] ?? null);
6769
$exporter = $this->registry->instantiate(
6870
$config['class'],
69-
false, // Churn exporters don't need config
70-
null
71+
$config['requiresConfig'] ?? false,
72+
$cognitiveConfig
7173
);
7274
$this->registry->validateInterface($exporter, ReportGeneratorInterface::class);
7375

‎src/Command/ChurnCommand.php‎

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,20 @@
44

55
namespace Phauthentic\CognitiveCodeAnalysis\Command;
66

7+
use Exception;
78
use Phauthentic\CognitiveCodeAnalysis\Business\CodeCoverage\CloverReader;
89
use Phauthentic\CognitiveCodeAnalysis\Business\CodeCoverage\CoberturaReader;
910
use Phauthentic\CognitiveCodeAnalysis\Business\CodeCoverage\CoverageReportReaderInterface;
1011
use Phauthentic\CognitiveCodeAnalysis\Business\MetricsFacade;
1112
use Phauthentic\CognitiveCodeAnalysis\CognitiveAnalysisException;
1213
use Phauthentic\CognitiveCodeAnalysis\Command\Handler\ChurnReportHandler;
1314
use Phauthentic\CognitiveCodeAnalysis\Command\Presentation\ChurnTextRenderer;
15+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\ChurnCommandContext;
16+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\CompositeChurnValidationSpecification;
17+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\CoverageFileExistsSpecification;
18+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\CoverageFormatExclusivitySpecification;
19+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\CoverageFormatSupportedSpecification;
20+
use Phauthentic\CognitiveCodeAnalysis\Command\ChurnSpecifications\ReportOptionsCompleteSpecification;
1421
use Symfony\Component\Console\Attribute\AsCommand;
1522
use Symfony\Component\Console\Command\Command;
1623
use Symfony\Component\Console\Input\InputArgument;
@@ -36,6 +43,8 @@ class ChurnCommand extends Command
3643
public const OPTION_COVERAGE_COBERTURA = 'coverage-cobertura';
3744
public const OPTION_COVERAGE_CLOVER = 'coverage-clover';
3845

46+
private CompositeChurnValidationSpecification $validationSpecification;
47+
3948
/**
4049
* Constructor to initialize dependencies.
4150
*/
@@ -45,6 +54,17 @@ public function __construct(
4554
readonly private ChurnReportHandler $report
4655
) {
4756
parent::__construct();
57+
$this->initializeValidationSpecification();
58+
}
59+
60+
private function initializeValidationSpecification(): void
61+
{
62+
$this->validationSpecification = new CompositeChurnValidationSpecification([
63+
new CoverageFormatExclusivitySpecification(),
64+
new CoverageFileExistsSpecification(),
65+
new CoverageFormatSupportedSpecification(),
66+
new ReportOptionsCompleteSpecification(),
67+
]);
4868
}
4969

5070
/**
@@ -118,61 +138,64 @@ protected function configure(): void
118138
*/
119139
protected function execute(InputInterface $input, OutputInterface $output): int
120140
{
121-
$coberturaFile = $input->getOption(self::OPTION_COVERAGE_COBERTURA);
122-
$cloverFile = $input->getOption(self::OPTION_COVERAGE_CLOVER);
141+
$context = new ChurnCommandContext($input);
123142

124-
// Validate that only one coverage option is specified
125-
if ($coberturaFile !== null && $cloverFile !== null) {
126-
$output->writeln('<error>Only one coverage format can be specified at a time.</error>');
143+
// Validate all specifications
144+
if (!$this->validationSpecification->isSatisfiedBy($context)) {
145+
$errorMessage = $this->validationSpecification->getDetailedErrorMessage($context);
146+
$output->writeln('<error>' . $errorMessage . '</error>');
127147
return self::FAILURE;
128148
}
129149

130-
$coverageFile = $coberturaFile ?? $cloverFile;
131-
$coverageFormat = $coberturaFile !== null ? 'cobertura' : ($cloverFile !== null ? 'clover' : null);
132-
133-
if (!$this->coverageFileExists($coverageFile, $output)) {
134-
return self::FAILURE;
150+
// Load configuration if provided
151+
if ($context->hasConfigFile()) {
152+
$configFile = $context->getConfigFile();
153+
if ($configFile !== null && !$this->loadConfiguration($configFile, $output)) {
154+
return self::FAILURE;
155+
}
135156
}
136157

137-
$coverageReader = $this->loadCoverageReader($coverageFile, $coverageFormat, $output);
158+
// Load coverage reader
159+
$coverageReader = $this->loadCoverageReader($context, $output);
138160
if ($coverageReader === false) {
139161
return self::FAILURE;
140162
}
141163

164+
// Calculate churn metrics
142165
$classes = $this->metricsFacade->calculateChurn(
143-
path: $input->getArgument(self::ARGUMENT_PATH),
144-
vcsType: $input->getOption(self::OPTION_VCS),
145-
since: $input->getOption(self::OPTION_SINCE),
166+
path: $context->getPath(),
167+
vcsType: $context->getVcsType(),
168+
since: $context->getSince(),
146169
coverageReader: $coverageReader
147170
);
148171

149-
$reportType = $input->getOption(self::OPTION_REPORT_TYPE);
150-
$reportFile = $input->getOption(self::OPTION_REPORT_FILE);
151-
152-
if ($reportType !== null || $reportFile !== null) {
153-
return $this->report->exportToFile($classes, $reportType, $reportFile);
172+
// Handle report generation or display
173+
if ($context->hasReportOptions()) {
174+
return $this->report->exportToFile(
175+
$classes,
176+
$context->getReportType(),
177+
$context->getReportFile()
178+
);
154179
}
155180

156-
$this->renderer->renderChurnTable(
157-
classes: $classes
158-
);
159-
181+
$this->renderer->renderChurnTable(classes: $classes);
160182
return self::SUCCESS;
161183
}
162184

163185
/**
164186
* Load coverage reader from file
165187
*
166-
* @param string|null $coverageFile Path to coverage file or null
167-
* @param string|null $format Coverage format ('cobertura', 'clover') or null for auto-detect
188+
* @param ChurnCommandContext $context Command context containing coverage file information
168189
* @param OutputInterface $output Output interface for error messages
169190
* @return CoverageReportReaderInterface|null|false Returns reader instance, null if no file provided, or false on error
170191
*/
171192
private function loadCoverageReader(
172-
?string $coverageFile,
173-
?string $format,
193+
ChurnCommandContext $context,
174194
OutputInterface $output
175195
): CoverageReportReaderInterface|null|false {
196+
$coverageFile = $context->getCoverageFile();
197+
$format = $context->getCoverageFormat();
198+
176199
if ($coverageFile === null) {
177200
return null;
178201
}
@@ -224,24 +247,22 @@ private function detectCoverageFormat(string $coverageFile): ?string
224247
return null;
225248
}
226249

227-
private function coverageFileExists(?string $coverageFile, OutputInterface $output): bool
228-
{
229-
// If no coverage file is provided, validation passes (backward compatibility)
230-
if ($coverageFile === null) {
231-
return true;
232-
}
233250

234-
// If coverage file is provided, check if it exists
235-
if (file_exists($coverageFile)) {
251+
/**
252+
* Loads configuration and handles errors.
253+
*
254+
* @param string $configFile
255+
* @param OutputInterface $output
256+
* @return bool Success or failure.
257+
*/
258+
private function loadConfiguration(string $configFile, OutputInterface $output): bool
259+
{
260+
try {
261+
$this->metricsFacade->loadConfig($configFile);
236262
return true;
263+
} catch (Exception $e) {
264+
$output->writeln('<error>Failed to load configuration: ' . $e->getMessage() . '</error>');
265+
return false;
237266
}
238-
239-
// Coverage file was provided but doesn't exist - show error
240-
$output->writeln(sprintf(
241-
'<error>Coverage file not found: %s</error>',
242-
$coverageFile
243-
));
244-
245-
return false;
246267
}
247268
}

0 commit comments

Comments
 (0)