Skip to content

Commit 2e9424b

Browse files
zero-to-prodzero-to-prod
authored
Improve describe attribute handling (#50)
* feat/improve-describe-attribute Updated `src/Describe.php`. * feat/improve-describe-attribute Created `tests/Unit/Describe/Attribute/AttributeTest.php`. * Refactor Describe attribute handling in DataModel.php Streamline attribute processing by reorganizing logic for methods and properties. Added a pre-computation step for property attributes to improve code clarity and maintainability. This enhances readability and prepares the code for potential future extensions.
1 parent 38fc6f0 commit 2e9424b

File tree

3 files changed

+264
-76
lines changed

3 files changed

+264
-76
lines changed

‎src/DataModel.php‎

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -181,44 +181,48 @@ public static function from(array|object|null|string $context = [], mixed $insta
181181
$methods = [];
182182
foreach ($ReflectionClass->getMethods() as $ReflectionMethod) {
183183
$ReflectionAttributes = $ReflectionMethod->getAttributes(Describe::class);
184-
if (!empty($ReflectionAttributes)) {
185-
foreach ($ReflectionAttributes as $ReflectionAttribute) {
186-
$property = $ReflectionAttribute->getArguments()[0];
187-
try {
188-
if (!isset($methods[$property])) {
189-
throw new ReflectionException();
190-
}
191-
$filename = $ReflectionClass->getMethod($methods[$property])->getFileName();
192-
$start_line = $ReflectionClass->getMethod($methods[$property])->getStartLine();
193-
} catch (ReflectionException) {
194-
$filename = null;
195-
$start_line = null;
184+
foreach ($ReflectionAttributes as $ReflectionAttribute) {
185+
$property = $ReflectionAttribute->getArguments()[0];
186+
try {
187+
if (!isset($methods[$property])) {
188+
throw new ReflectionException();
196189
}
197-
$methods[$property] = isset($methods[$property])
198-
? throw new DuplicateDescribeAttributeException(
199-
sprintf(
200-
"\nDuplicate #[Describe($property)] attribute for property $%s found in methods:\n".
201-
"%s() %s:%d\n".
202-
"%s() %s:%d",
203-
$property,
204-
$methods[$property],
205-
$filename,
206-
$start_line,
207-
$ReflectionMethod->getName(),
208-
$ReflectionMethod->getFileName(),
209-
$ReflectionMethod->getStartLine()
210-
)
211-
)
212-
: $ReflectionMethod->getName();
190+
$filename = $ReflectionClass->getMethod($methods[$property])->getFileName();
191+
$start_line = $ReflectionClass->getMethod($methods[$property])->getStartLine();
192+
} catch (ReflectionException) {
193+
$filename = null;
194+
$start_line = null;
213195
}
196+
$methods[$property] = isset($methods[$property])
197+
? throw new DuplicateDescribeAttributeException(
198+
sprintf(
199+
"\nDuplicate #[Describe($property)] attribute for property $%s found in methods:\n".
200+
"%s() %s:%d\n".
201+
"%s() %s:%d",
202+
$property,
203+
$methods[$property],
204+
$filename,
205+
$start_line,
206+
$ReflectionMethod->getName(),
207+
$ReflectionMethod->getFileName(),
208+
$ReflectionMethod->getStartLine()
209+
)
210+
)
211+
: $ReflectionMethod->getName();
214212
}
215213
}
216214

215+
$propertyAttributes = [];
216+
$ReflectionProperties = $ReflectionClass->getProperties();
217+
foreach ($ReflectionProperties as $ReflectionProperty) {
218+
$propertyAttributes[$ReflectionProperty->getName()] =
219+
$ReflectionProperty->getAttributes(Describe::class)[0] ?? null;
220+
}
221+
217222
$context = is_object($context) ? (array)$context : $context ?? [];
218223

219-
foreach ($ReflectionClass->getProperties() as $ReflectionProperty) {
220-
$Attribute = ($ReflectionProperty->getAttributes(Describe::class)[0] ?? null);
221-
/** @var Describe $Describe */
224+
foreach ($ReflectionProperties as $ReflectionProperty) {
225+
$Attribute = $propertyAttributes[$ReflectionProperty->getName()];
222226
$Describe = $Attribute?->newInstance();
223227

224228
if (isset($Describe->ignore) && $Describe->ignore) {

‎src/Describe.php‎

Lines changed: 85 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,32 @@
103103
class Describe
104104
{
105105
public const missing_as_null = 'missing_as_null';
106-
public const nullable = 'nullable';
107-
public const required = 'required';
108-
public const ignore = 'ignore';
109-
106+
/** @see $from */
107+
public const from = 'from';
110108
public string $from;
111-
109+
/** @see $cast */
110+
public const cast = 'cast';
112111
public string|array $cast;
113-
112+
/** @see $required */
113+
public const required = 'required';
114114
public bool $required;
115-
115+
/** @see $default */
116+
public const default = 'default';
116117
public $default;
117-
118+
/** @see $pre */
119+
public const pre = 'pre';
118120
public $pre;
119-
121+
/** @see $post */
122+
public const post = 'post';
120123
public $post;
121-
124+
/** @see $nullable */
125+
public const nullable = 'nullable';
122126
public bool $nullable;
123-
127+
/** @see $ignore */
128+
public const ignore = 'ignore';
124129
public bool $ignore;
125-
130+
/** @see $via */
131+
public const via = 'via';
126132
public string|array $via;
127133

128134
/**
@@ -224,51 +230,85 @@ class Describe
224230
*/
225231
public function __construct(string|null|array $attributes = null)
226232
{
227-
if (is_countable($attributes)) {
228-
foreach ($attributes as $key => $value) {
229-
if (property_exists($this, $key)) {
230-
if ($key === self::nullable && !is_bool($value)) {
231-
throw new InvalidValue(sprintf("Invalid value: `%s` should be a boolean.", self::nullable));
232-
}
233+
if (!is_array($attributes)) {
234+
return;
235+
}
233236

234-
if ($key === self::required && !is_bool($value)) {
235-
throw new InvalidValue(sprintf("Invalid value: `%s` should be a boolean.", self::required));
237+
foreach ($attributes as $key => $value) {
238+
switch ($key) {
239+
case self::required:
240+
if (!is_bool($value)) {
241+
throw new InvalidValue('Invalid value: `required` should be a boolean.');
236242
}
243+
$this->required = $value;
244+
break;
237245

238-
if ($key === self::ignore && !is_bool($value)) {
239-
throw new InvalidValue(sprintf("Invalid value: `%s` should be a boolean.", self::ignore));
246+
case self::nullable:
247+
if (!is_bool($value)) {
248+
throw new InvalidValue('Invalid value: `nullable` should be a boolean.');
240249
}
250+
$this->nullable = $value;
251+
break;
241252

242-
$this->$key = $value;
243-
continue;
244-
}
245-
if (is_string($value) && property_exists($this, $value) && $key === 0) {
246-
if ($value === self::required) {
247-
$this->$value = true;
253+
case self::ignore:
254+
if (!is_bool($value)) {
255+
throw new InvalidValue('Invalid value: `ignore` should be a boolean.');
248256
}
257+
$this->ignore = $value;
258+
break;
249259

250-
if ($value === self::nullable) {
251-
$this->$value = true;
260+
case self::missing_as_null:
261+
if (!is_bool($value)) {
262+
throw new InvalidValue('Invalid value: `missing_as_null` should be a boolean.');
252263
}
264+
$this->nullable = $value;
265+
break;
266+
267+
case self::from:
268+
$this->from = $value;
269+
break;
253270

254-
if ($value === self::ignore) {
255-
$this->$value = true;
271+
case self::cast:
272+
$this->cast = $value;
273+
break;
274+
275+
case self::default:
276+
$this->default = $value;
277+
break;
278+
279+
case self::pre:
280+
$this->pre = $value;
281+
break;
282+
283+
case self::post:
284+
$this->post = $value;
285+
break;
286+
287+
case self::via:
288+
$this->via = $value;
289+
break;
290+
291+
case 0:
292+
if (is_string($value)) {
293+
switch ($value) {
294+
case self::required:
295+
$this->required = true;
296+
break;
297+
case self::missing_as_null:
298+
case self::nullable:
299+
$this->nullable = true;
300+
break;
301+
case self::ignore:
302+
$this->ignore = true;
303+
break;
304+
}
256305
}
257-
}
258-
/**
259-
* Aliases
260-
*/
306+
break;
261307

262-
if ($key === self::missing_as_null) {
263-
if (!is_bool($value)) {
264-
throw new InvalidValue(sprintf("Invalid value: `%s` should be a boolean.", self::missing_as_null));
308+
default:
309+
if ($value === self::missing_as_null) {
310+
$this->nullable = true;
265311
}
266-
$this->nullable = $value;
267-
continue;
268-
}
269-
if ($value === self::missing_as_null) {
270-
$this->nullable = true;
271-
}
272312
}
273313
}
274314
}

‎tests/Unit/Describe/Attribute/AttributeTest.php‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
<?php
2+
3+
namespace Tests\Unit\Describe\Attribute;
4+
5+
use Tests\TestCase;
6+
use Zerotoprod\DataModel\Describe;
7+
use Zerotoprod\DataModel\InvalidValue;
8+
9+
class AttributeTest extends TestCase
10+
{
11+
public function testRequiredPositive(): void
12+
{
13+
$d = new Describe(['required' => true]);
14+
$this->assertTrue($d->required);
15+
}
16+
17+
public function testRequiredNegative(): void
18+
{
19+
$this->expectException(InvalidValue::class);
20+
new Describe(['required' => 'yes']);
21+
}
22+
23+
public function testNullablePositive(): void
24+
{
25+
$d = new Describe(['nullable' => false]);
26+
$this->assertFalse($d->nullable);
27+
}
28+
29+
public function testNullableNegative(): void
30+
{
31+
$this->expectException(InvalidValue::class);
32+
new Describe(['nullable' => 'nope']);
33+
}
34+
35+
public function testIgnorePositive(): void
36+
{
37+
$d = new Describe(['ignore' => true]);
38+
$this->assertTrue($d->ignore);
39+
}
40+
41+
public function testIgnoreNegative(): void
42+
{
43+
$this->expectException(InvalidValue::class);
44+
new Describe(['ignore' => 1]);
45+
}
46+
47+
public function testMissingAsNullPositive(): void
48+
{
49+
$d = new Describe([Describe::missing_as_null => true]);
50+
$this->assertTrue($d->nullable);
51+
}
52+
53+
public function testMissingAsNullNegative(): void
54+
{
55+
$this->expectException(InvalidValue::class);
56+
new Describe([Describe::missing_as_null => 'abc']);
57+
}
58+
59+
public function testFromPositive(): void
60+
{
61+
$d = new Describe(['from' => 'user_name']);
62+
$this->assertSame('user_name', $d->from);
63+
}
64+
65+
public function testFromNegative(): void
66+
{
67+
// Should not throw error, just not assign for wrong key
68+
$d = new Describe(['form' => 'typo']);
69+
$this->assertFalse(isset($d->form));
70+
$this->assertNull(@$d->form);
71+
}
72+
73+
public function testCastPositive(): void
74+
{
75+
$d = new Describe(['cast' => 'int']);
76+
$this->assertSame('int', $d->cast);
77+
}
78+
79+
public function testDefaultPositive(): void
80+
{
81+
$d = new Describe(['default' => 42]);
82+
$this->assertSame(42, $d->default);
83+
}
84+
85+
public function testPrePositive(): void
86+
{
87+
$d = new Describe(['pre' => 'trim']);
88+
$this->assertSame('trim', $d->pre);
89+
}
90+
91+
public function testPostPositive(): void
92+
{
93+
$d = new Describe(['post' => 'strtoupper']);
94+
$this->assertSame('strtoupper', $d->post);
95+
}
96+
97+
public function testViaPositive(): void
98+
{
99+
$d = new Describe(['via' => 'email']);
100+
$this->assertSame('email', $d->via);
101+
}
102+
103+
public function testShortcutRequiredPositive(): void
104+
{
105+
$d = new Describe([Describe::required]);
106+
$this->assertTrue($d->required);
107+
}
108+
109+
public function testShortcutNullablePositive(): void
110+
{
111+
$d = new Describe([Describe::nullable]);
112+
$this->assertTrue($d->nullable);
113+
}
114+
115+
public function testShortcutIgnorePositive(): void
116+
{
117+
$d = new Describe([Describe::ignore]);
118+
$this->assertTrue($d->ignore);
119+
}
120+
121+
public function testShortcutMissingAsNullPositive(): void
122+
{
123+
$d = new Describe([Describe::missing_as_null]);
124+
$this->assertTrue($d->nullable);
125+
}
126+
127+
public function testShortcutWrongKeyNegative(): void
128+
{
129+
$d = new Describe([0 => 'not_a_known_alias']);
130+
$this->assertFalse(isset($d->not_a_known_alias));
131+
}
132+
133+
public function testMissingAsNullAsValuePositive(): void
134+
{
135+
$d = new Describe(['yep' => Describe::missing_as_null]);
136+
$this->assertTrue($d->nullable);
137+
}
138+
139+
public function testNullInput(): void
140+
{
141+
$d = new Describe();
142+
$this->assertInstanceOf(Describe::class, $d);
143+
}
144+
}

0 commit comments

Comments
 (0)