Skip to content

Commit 4f81c8a

Browse files
othercodesothercodes
authored
Fix/reduce cognitive complexity and improve exception handling in IsModel (#24)
* Reduce cognitive complexity and improve exception handling in IsModel * Add unit tests for HasEquality and HasImmutability traits
1 parent 6dc84ee commit 4f81c8a

13 files changed

+546
-126
lines changed

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@
4040
}
4141
},
4242
"scripts": {
43-
"test": "vendor/bin/pest --configuration=phpunit.xml --coverage-clover=coverage.xml --log-junit=test.xml",
44-
"test-cov": "vendor/bin/pest --configuration=phpunit.xml --coverage-html=coverage",
43+
"test": "vendor/bin/pest --configuration=phpunit.xml --coverage --coverage-clover=coverage.xml --log-junit=test.xml",
44+
"test-cov": "vendor/bin/pest --configuration=phpunit.xml --coverage --coverage-html=coverage",
4545
"analyse": "vendor/bin/phpstan analyse src --no-progress --memory-limit=4G --level=8",
4646
"check": [
4747
"@analyse",

‎src/IsModel.php‎

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
use ReflectionClass;
1111
use ReflectionMethod;
1212
use ReflectionNamedType;
13+
use ReflectionParameter;
1314
use ReflectionUnionType;
14-
use RuntimeException;
15+
use ComplexHeart\Domain\Model\Exceptions\InstantiationException;
1516
use TypeError;
1617

1718
/**
@@ -51,7 +52,7 @@ final public static function make(mixed ...$params): static
5152
$constructor = $reflection->getConstructor();
5253

5354
if (!$constructor) {
54-
throw new RuntimeException(
55+
throw new InstantiationException(
5556
sprintf('%s must have a constructor to use make()', static::class)
5657
);
5758
}
@@ -121,46 +122,54 @@ private static function validateConstructorParameters(
121122

122123
// Validate types for each parameter
123124
foreach ($constructorParams as $index => $param) {
124-
if (!isset($params[$index])) {
125-
continue; // Optional parameter not provided
125+
if (isset($params[$index])) {
126+
self::validateParameterType($param, $params[$index]);
126127
}
128+
}
129+
}
127130

128-
$value = $params[$index];
129-
$type = $param->getType();
130131

131-
if ($type === null) {
132-
continue; // No type hint
133-
}
132+
/**
133+
* Validate a single parameter's type.
134+
*
135+
* @param ReflectionParameter $param
136+
* @param mixed $value
137+
* @return void
138+
* @throws TypeError
139+
*/
140+
private static function validateParameterType(ReflectionParameter $param, mixed $value): void
141+
{
142+
$type = $param->getType();
134143

135-
$isValid = false;
136-
$expectedTypes = '';
137-
138-
if ($type instanceof ReflectionNamedType) {
139-
// Single type
140-
$isValid = self::validateType($value, $type->getName(), $type->allowsNull());
141-
$expectedTypes = $type->getName();
142-
} elseif ($type instanceof ReflectionUnionType) {
143-
// Union type (e.g., int|float|string)
144-
$isValid = self::validateUnionType($value, $type);
145-
$expectedTypes = implode('|', array_map(
146-
fn ($t) => $t instanceof ReflectionNamedType ? $t->getName() : 'mixed',
147-
$type->getTypes()
148-
));
149-
} else {
150-
continue; // Intersection types or other complex types not supported yet
151-
}
144+
if ($type === null) {
145+
return; // No type hint
146+
}
152147

153-
if (!$isValid) {
154-
throw new TypeError(
155-
sprintf(
156-
'%s::make() parameter "%s" must be of type %s, %s given',
157-
basename(str_replace('\\', '/', static::class)),
158-
$param->getName(),
159-
$expectedTypes,
160-
get_debug_type($value)
161-
)
162-
);
163-
}
148+
if ($type instanceof ReflectionNamedType) {
149+
// Single type
150+
$isValid = self::validateType($value, $type->getName(), $type->allowsNull());
151+
$expectedTypes = $type->getName();
152+
} elseif ($type instanceof ReflectionUnionType) {
153+
// Union type (e.g., int|float|string)
154+
$isValid = self::validateUnionType($value, $type);
155+
$expectedTypes = implode('|', array_map(
156+
fn ($t) => $t instanceof ReflectionNamedType ? $t->getName() : 'mixed',
157+
$type->getTypes()
158+
));
159+
} else {
160+
return; // Intersection types or other complex types not supported yet
161+
}
162+
163+
if (!$isValid) {
164+
throw new TypeError(
165+
sprintf(
166+
'%s::make() parameter "%s" must be of type %s, %s given',
167+
basename(str_replace('\\', '/', static::class)),
168+
$param->getName(),
169+
$expectedTypes,
170+
get_debug_type($value)
171+
)
172+
);
164173
}
165174
}
166175

‎src/Traits/HasImmutability.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ trait HasImmutability
2626
* @param mixed $_
2727
* @return void
2828
*/
29-
final public function __set(string $name, $_): void
29+
final public function __set(string $name, mixed $_): void
3030
{
3131
$class = static::class;
3232
throw new ImmutabilityError("Cannot modify property $name from immutable $class object.");

‎src/Traits/HasTypeCheck.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,17 @@ protected function isValueTypeValid(mixed $value, string $validType): bool
2626
return true;
2727
}
2828

29-
$primitives = ['integer', 'boolean', 'float', 'string', 'array', 'object', 'callable'];
29+
// Handle float/double equivalence (PHP uses 'double' internally)
30+
if ($validType === 'float') {
31+
$validType = 'double';
32+
}
33+
34+
// Handle callable type check
35+
if ($validType === 'callable') {
36+
return is_callable($value);
37+
}
38+
39+
$primitives = ['integer', 'boolean', 'double', 'string', 'array', 'object'];
3040
$validation = in_array($validType, $primitives)
3141
? fn ($value): bool => gettype($value) === $validType
3242
: fn ($value): bool => $value instanceof $validType;

‎tests/AggregatesTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
$order2 = Order::create(1, ['id' => UUIDValue::random(), 'name' => 'Vincent Vega']);
2626
$order3 = Order::create(2, ['id' => UUIDValue::random(), 'name' => 'Vincent Vega']);
2727

28-
expect($order1->equals($order2))->toBeTrue();
29-
expect($order1->equals($order3))->toBeFalse();
30-
expect($order1->equals(new stdClass()))->toBeFalse();
28+
expect($order1->equals($order2))->toBeTrue()
29+
->and($order1->equals($order3))->toBeFalse()
30+
->and($order1->equals(new stdClass()))->toBeFalse();
3131
})
3232
->group('Unit');
3333

‎tests/AutoCheckInvariantsTest.php‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎tests/HasEqualityTest.php‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use ComplexHeart\Domain\Model\Traits\HasEquality;
6+
7+
test('HasEquality should compare objects by hash', function () {
8+
$class = get_class(new class ('dummy') {
9+
use HasEquality;
10+
11+
public function __construct(private string $value)
12+
{
13+
}
14+
15+
public function __toString(): string
16+
{
17+
return $this->value;
18+
}
19+
});
20+
21+
$obj1 = new $class('test');
22+
$obj2 = new $class('test');
23+
24+
expect($obj1->equals($obj2))->toBeTrue();
25+
})
26+
->group('Unit');
27+
28+
test('HasEquality should return false for different string representations', function () {
29+
$obj1 = new class ('test1') {
30+
use HasEquality;
31+
32+
public function __construct(private string $value)
33+
{
34+
}
35+
36+
public function __toString(): string
37+
{
38+
return $this->value;
39+
}
40+
};
41+
42+
$obj2 = new class ('test2') {
43+
use HasEquality;
44+
45+
public function __construct(private string $value)
46+
{
47+
}
48+
49+
public function __toString(): string
50+
{
51+
return $this->value;
52+
}
53+
};
54+
55+
expect($obj1->equals($obj2))->toBeFalse();
56+
})
57+
->group('Unit');
58+
59+
test('HasEquality should return false for different class types', function () {
60+
$obj1 = new class ('test') {
61+
use HasEquality;
62+
63+
public function __construct(private string $value)
64+
{
65+
}
66+
67+
public function __toString(): string
68+
{
69+
return $this->value;
70+
}
71+
};
72+
73+
$obj2 = new stdClass();
74+
75+
expect($obj1->equals($obj2))->toBeFalse();
76+
})
77+
->group('Unit');
78+
79+
test('HasEquality::hash should generate consistent SHA256 hash', function () {
80+
$obj = new class ('test-value') {
81+
use HasEquality;
82+
83+
public function __construct(private string $value)
84+
{
85+
}
86+
87+
public function __toString(): string
88+
{
89+
return $this->value;
90+
}
91+
92+
public function getHash(): string
93+
{
94+
return $this->hash();
95+
}
96+
};
97+
98+
$expectedHash = hash('sha256', 'test-value');
99+
100+
expect($obj->getHash())->toBe($expectedHash);
101+
})
102+
->group('Unit');
103+
104+
test('HasEquality should handle complex objects', function () {
105+
$class = get_class(new class (['dummy' => 'data']) {
106+
use HasEquality;
107+
108+
public function __construct(private array $data)
109+
{
110+
}
111+
112+
public function __toString(): string
113+
{
114+
return json_encode($this->data);
115+
}
116+
});
117+
118+
$obj1 = new $class(['name' => 'John', 'age' => 30]);
119+
$obj2 = new $class(['name' => 'John', 'age' => 30]);
120+
$obj3 = new $class(['name' => 'Jane', 'age' => 25]);
121+
122+
expect($obj1->equals($obj2))->toBeTrue()
123+
->and($obj1->equals($obj3))->toBeFalse();
124+
})
125+
->group('Unit');
126+
127+
test('HasEquality should handle empty strings', function () {
128+
$class = get_class(new class ('dummy') {
129+
use HasEquality;
130+
131+
public function __construct(private string $value)
132+
{
133+
}
134+
135+
public function __toString(): string
136+
{
137+
return $this->value;
138+
}
139+
});
140+
141+
$obj1 = new $class('');
142+
$obj2 = new $class('');
143+
144+
expect($obj1->equals($obj2))->toBeTrue();
145+
})
146+
->group('Unit');

‎tests/HasImmutabilityTest.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use ComplexHeart\Domain\Model\Errors\ImmutabilityError;
6+
use ComplexHeart\Domain\Model\Test\Fixtures\OrderManagement\Domain\Price;
7+
8+
test('Object with HasImmutability should throw ImmutabilityError for any update properties attempts.', function () {
9+
$price = new Price(100.0, 'EUR');
10+
$price->amount = 0.0;
11+
})
12+
->group('Unit')
13+
->throws(ImmutabilityError::class);
14+
15+
test('Object with HasImmutability should expose primitive values.', function () {
16+
$price = new Price(100.0, 'EUR');
17+
expect($price->amount)->toBeFloat()
18+
->and($price->currency)->toBeString();
19+
})
20+
->group('Unit');
21+
22+
test('Object with HasImmutability should return new instance with override values.', function () {
23+
$price = new Price(100.0, 'EUR');
24+
$newPrice = $price->applyDiscount(10.0);
25+
26+
expect($newPrice)->toBeInstanceOf(Price::class)
27+
->and($newPrice->amount)->toBe(90.0);
28+
})
29+
->group('Unit');

0 commit comments

Comments
 (0)