Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nepovinný parametr #4

Open
meridius opened this issue Feb 27, 2017 · 9 comments
Open

Nepovinný parametr #4

meridius opened this issue Feb 27, 2017 · 9 comments

Comments

@meridius
Copy link
Contributor

Ahoj,

řeším problém s nepovinným parametrem a zjištěním jeho stavu.
Mějme např. následující strukturu, kde otherData může být pole hodnot, ale může být i null nebo prázdné pole.

$data = [
	'id' => 42,
	'name' => 'Tomas Marny',
	'otherData' => [
		'data1' => 'foo',
		'data2' => 'bar',
	],
];

$data = [
	'id' => 42,
	'name' => 'Tomas Marny',
	'otherData' => [],
];

Jak v tomto případě zjistím stav "prázdnosti", resp. že otherData nemá žádné property, na které by se dalo přistupovat?
Pokud totiž entitu nadefinuji následovně, tak typ property otherData bude vždy OtherData, nehledě na vstup.

/**
 *
 * @property-read int id
 * @property-read string name
 * @property-read OtherData otherData
 */
class Data extends Entry
{
    protected static $associations = [
        'otherData' => OtherData::class,
    ];
}

/**
 *
 * @property-read string data1
 * @property-read string data2
 */
class OtherData extends Entry
{
}
@stekycz
Copy link

stekycz commented Feb 27, 2017

Teď, ať koukám jak koukám, to neuděláš. Každopádně dobrý point. Pro null by se mi líbil asi podobný zápis

/**
 * @property-read OtherData|null $otherData
 */
class Data extends Entry
{
    protected static $associations = [
        'otherData' => new NullableAssociation(OtherData::class),
    ];
}

S tím že by sis mohl nadefinovat svoje podobné požadavky (se stejným interface), aby se jako prázdné vyhodnotilo i prázdné pole (za mě výchozí null v rámci knihovny je ok).

@stekycz
Copy link

stekycz commented Feb 27, 2017

Pravděpodobně by se pak upravovalo vytváření nových objektů. Možná by se tak nechaly inicializovat i kolekce? Něco jako

/**
 * @property-read OtherData[] $otherData
 */
class Data extends Entry
{
    protected static $associations = [
        'otherData' => new ListAssociation(OtherData::class),
    ];
}

A případně to kombinovat (pořadí důležité, jinak by mohly být nullable prvky té kolekce)

/**
 * @property-read OtherData[]|null $otherData
 */
class Data extends Entry
{
    protected static $associations = [
        'otherData' => new NullableAssociation(new ListAssociation(OtherData::class)),
    ];
}

@stekycz
Copy link

stekycz commented Feb 27, 2017

Po nějakém testování pokud přijde jako hodnota null, tak to zůstane null a entita se nevytváří. Viz kód. Nicméně rozhodnutý o jiných falsey hodnotách se tam neřeší.

Zároveň do statického pole nevytvoříme novou instanci, jak uvažuji výše. Navrhl bych tedy do syntaxe názvu asociace přidat ?, pokud se má vyhodnotovat na prázdnost pomocí == namísto ===. Něco jako

/**
 * @property-read OtherData|null $otherData
 */
class Data extends Entry
{
    protected static $associations = [
        '?otherData' => OtherData::class,
    ];
}

Což trochu kopíruje syntaxy nullable hodnot v rámci PHP 7.1 :-)

@meridius
Copy link
Contributor Author

meridius commented Mar 1, 2017

@stekycz To vypadá super! A kde to zkouším, tam to funguje. ;)
@Tharos Neuvažuješ o merge?

@Tharos
Copy link
Owner

Tharos commented Mar 1, 2017

Uvažuju :), jenom chci věc doplnit i do docky. Prosím jen o malinké strpení…

@stekycz
Copy link

stekycz commented Mar 1, 2017

Přemýšlím ještě o dalších testech na doplnění a případné úpravy, jak to funguje.

  • Co má být výsledkem pro zápis ?orderItems[]? Má to být null nebo []? Za mě [], ale to teď asi nezafunguje. Tzn. napsat na to test.
  • Rozšířit testy na další falsey hodnoty, tzn. false, 0, 0.0, null, '' - má to vůbec smysl?
  • Jak má fungovat třeba ?author.a_? Musí být otazník u každého atributu a všechny musí být empty, aby se to aplikovalo na celého authora? Například
$book = new Book([
	'id' => 12,
	'title' => 'PHP: The Bad Parts',
	'tag_name' => 'bestseller',
	'customer_id' => 20,
	'a_firstname' => null,
	'a_surname' => null,
]);

povede na to, že $book->author === null?

@Tharos
Copy link
Owner

Tharos commented Mar 1, 2017

To s tím ?orderItems[] je docela good point, protože ono už teď se Schematic chová tak, že pro NULL v takovém případě vrátí NULL. Na čemž je vidět, že jsem to nikdy moc dopodrobna nerozmýšlel… :)

Hezké by asi bylo, aby se vždy vrátila instance IEntries (což by teoreticky byl BC break), a také si říkám, že definice s ? by mohla pro různé falsey hodnoty vracet IEntries obalující prázdné pole.

Co myslíš?


U embedded záznamů je trochu prekérka tohle: https://github.com/Tharos/Schematic/blob/develop/src/Schematic/Entry.php#L152 Nenapadl mě žádný jiný způsob, jak to principiálně lépe vyřešit…

Proteď bych možná šel cestou ? pro embedded záznamy uměle zakázat. Tak, jako je nyní zakázané kombinovat embedded záznamy s kolekcemi (například tag[]. skončí výjimkou).

@meridius
Copy link
Contributor Author

meridius commented Mar 1, 2017

Jestli můžu ...

Pokud se bavíme o případu, kdy je null field zapsaný jako ?orderItems[], tak by se mělo vracet null a nic jiného. Jinak tam ? podle mě nemá smysl.
Pokud ale bude zapsaný jen jako orderItems[], tak samozřejmě vracet IEntries.

Jestli ne/rozšiřovat na další falsey hodnoty je otázka. 🤔

@stekycz
Copy link

stekycz commented Mar 2, 2017

Rozšířil jsem testy, udělal rebase a zdá se, že vše funguje tak jak bych očekával. Mrkněte na to :)

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

No branches or pull requests

3 participants