Implemented nullable association using empty #5
Conversation
|
Very nice solution, thank you! I'm going to push doc update and after that let's merge this. :) |
stekycz
force-pushed
the
nullable-association
branch
from
16ea2bc to
5f49ba6
Compare
Tharos
force-pushed
the
nullable-association
branch
3 times, most recently
from
3d20766 to
12a6252
Compare
|
Dovolil jsem si to rebasnout s ohledem na #6. Autorství úprav jsem samozřejmě zachoval. :) |
|
Stihl jsem udělat už předtím, ale dobře :P |
stekycz
force-pushed
the
nullable-association
branch
from
12a6252 to
330005d
Compare
src/Schematic/Entry.php
Outdated
| if ($data === NULL) { | ||
| if (!$association[self::INDEX_MULTIPLICITY] && ( | ||
| $data === NULL | ||
| || ($association[self::INDEX_NULLABLE] && empty($data)) |
There was a problem hiding this comment.
Je to drobnost, ale tady používáš space, místo TAB.
There was a problem hiding this comment.
A koukám, že jinde občas taky.
stekycz
force-pushed
the
nullable-association
branch
from
330005d to
5448ab7
Compare
stekycz
force-pushed
the
nullable-association
branch
from
5448ab7 to
e2b8b1e
Compare
| @@ -11,6 +11,7 @@ class Entry | |||
| const INDEX_ENTRYCLASS = 0; | |||
| const INDEX_MULTIPLICITY = 1; | |||
| const INDEX_EMBEDDING = 2; | |||
| const INDEX_NULLABLE = 3; | |||
There was a problem hiding this comment.
Konstanta INDEX_NULLABLE by měla být v doc pro self::$parsedAssociations.
tests/SchematicTests/EntryTest.phpt
Outdated
|
|
||
| Assert::type(CustomEntries::class, $firstOrderItem->tags); | ||
| Assert::type(CustomEntries::class, $lastOrderItem->tags); |
There was a problem hiding this comment.
Tohle se mi nezdá. Přece pokud mám $lastOrderItem->tags definované jako nullable multiple a plním ho "empty" hodnotami, tak by mělo vracet NULL.
Nevím, jaký má smysl nullable ne-multiple fileld, ale pokud nepřekáží, tak by měl pro empty hodnoty asi vracet NULL. Pokud uživatele budou empty hodnoty zajímat, tak by takový field neměl označovat jako nullable.
Pro přehlednost tabulka, jak si myslím, že by to mělo být:
| normal | nullable | multiple | nullable multiple | |
|---|---|---|---|---|
| text | text | text | TypeError | TypeError |
| number | number | number | TypeError | TypeError |
| data array | Entry | Entry | Entries | Entries |
| empty(value) | empty(value) | null | TypeError | null |
| null | null | null | TypeError | null |
| empty array | empty array | null | Entries | null |
| 0 | 0 | null | TypeError | null |
There was a problem hiding this comment.
Hmm, na tom něco bude. Nechal jsem se asi příliš unést tím, že nutím svůj přístup, že každá kolekce by měla být vždy kolekce a nikdy null, ale to by si měl člověk zvolit podle toho, co tam chce mít. Upravím.
tests/SchematicTests/EntryTest.phpt
Outdated
| } | ||
|
|
||
|
|
||
| public function testEmbeddedEntries() | ||
| public function provideDataNullableCollection() |
There was a problem hiding this comment.
Vzhledem k tomu, že to vrací stejné "empty" hodnoty jako provideDataNullableRelation(), tak bych oboje sjednotil pod něco jako provideDataNullableValues() nebo tak.
| FALSE, | ||
| self::INDEX_NULLABLE => !empty($matches[1]), |
There was a problem hiding this comment.
Tady bych možná empty() nahradil výsledkem volání protected funkce isEmpty() (nebo tak nějak), aby si uživatel měl možnost nastavit, jaké hodnoty považuje za ty správně "empty". Dovedu si představit, že asi ne všem se trefíme do use-case.
src/Schematic/Entry.php
Outdated
| @@ -101,15 +103,18 @@ public function __get($name) | |||
| $this->readEmbeddedEntry($association[self::INDEX_EMBEDDING]) : | |||
| $this->readData($name); | |||
|
|
|||
| if ($data === NULL) { | |||
| if (!$association[self::INDEX_MULTIPLICITY] && ( | |||
There was a problem hiding this comment.
Banalita, ale nechtělo by takhle složitý if učesat třeba takhle?
if (
!$association[self::INDEX_MULTIPLICITY]
&& (
$data === NULL
|| ($association[self::INDEX_NULLABLE] && empty($data))
)
) {
|
Upraveno dle komentářů od @meridius a rozšířeny testy snad na všechny možné kombinace hodnot a nastavení properties a asociací. Jen mi tam trochu nesedí to, že jakmile je hodnota Tak jen přemýšlím o přidání |
|
Ahoj vespolek, jenom chci dát vědět, že tenhle pull budu řešit ASAP… Mám nějaký zdravotní trable, ale jakmile mi bude líp, vše projdu. Snad už tento týden. Díky za trpělivost! :) |
|
@Tharos Děkuju za update a držím palce, ať je ti brzo líp. |
Refs #4