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

[WIP] Entity Event #12

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

[WIP] Entity Event #12

wants to merge 9 commits into from

Conversation

sukovec
Copy link
Member

@sukovec sukovec commented Feb 9, 2018

This change is Reviewable

@sukovec sukovec changed the title [WIP] Entity [WIP] Entity Event Feb 9, 2018
@michalhosna
Copy link
Contributor

Sorry že vám kecám už do WIPU :-D


Reviewed 4 of 7 files at r1.
Review status: 4 of 7 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.


src/Entity/Event.php, line 19 at r2 (raw file):

	 * @ORM\Column(type="Uuid", unique=true)
	 * @ORM\GeneratedValue(strategy="CUSTOM")
	 * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")

Takhle to sice najdete v DOCu, ale takhle dostane entita ID až při persistu, takžé člověk přijde o jednu z výhod UUID, že existuje nezávisle na DB enginu. Navíc takhle je ID vlastně nullable.

Ideálně používejte příštup co mám v example, tzn. ID generujete v constructoru. (Když se entita načíťá z DB tak se konstruktor nepoužívá o to se nemusíte bát._


src/Entity/Event.php, line 21 at r2 (raw file):

	 * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")
	 */
	protected $id;

Tedy je otázka jestli psáť propery protected nebo private.

Když pak dědíš tedy entitu rozšířuješ, tak bys nutně rodiče měnit nemusle, měl bys u value objectu jen rozšiřovat. Tzn. není nutné aby bylo protected.


src/Entity/Event.php, line 51 at r2 (raw file):

	public function __construct() {
	}

getter na ID missing.


src/Entity/EventType.php, line 16 at r2 (raw file):

	 * @ORM\Id
	 * @ORM\Column(type="integer")
	 * @ORM\GeneratedValue

Tohle už vůbec nebude fungovat.


src/Entity/EventType.php, line 38 at r2 (raw file):

		$this->name = $name;
	}

getter na ID


src/Entity/SubEvent.php, line 19 at r2 (raw file):

	 * @ORM\Column(type="Uuid", unique=true)
	 * @ORM\GeneratedValue(strategy="CUSTOM")
	 * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")

Viz výše.


src/Entity/SubEvent.php, line 34 at r2 (raw file):

	 * @var string
	 *
	 * @ORM\Column(type="string")

type="text"


Comments from Reviewable

@sukovec
Copy link
Member Author

sukovec commented Feb 11, 2018

Review status: 4 of 7 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.


src/Entity/EventType.php, line 16 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

Tohle už vůbec nebude fungovat.

Copypasta z dokumentace nebude fungovat? :-O Huh :)


src/Entity/EventType.php, line 38 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

getter na ID

getter na ID? V tvem examplu nebyl, tak jsem myslel, ze to je umyslne ;)


Comments from Reviewable

@sukovec
Copy link
Member Author

sukovec commented Feb 12, 2018

Review status: 4 of 7 files reviewed at latest revision, 7 unresolved discussions.


src/Entity/Event.php, line 19 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

Takhle to sice najdete v DOCu, ale takhle dostane entita ID až při persistu, takžé člověk přijde o jednu z výhod UUID, že existuje nezávisle na DB enginu. Navíc takhle je ID vlastně nullable.

Ideálně používejte příštup co mám v example, tzn. ID generujete v constructoru. (Když se entita načíťá z DB tak se konstruktor nepoužívá o to se nemusíte bát._

Done.


src/Entity/Event.php, line 51 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

getter na ID missing.

Done.


src/Entity/EventType.php, line 16 at r2 (raw file):

Previously, sukovec (Mike S.) wrote…

Copypasta z dokumentace nebude fungovat? :-O Huh :)

Done.


src/Entity/EventType.php, line 38 at r2 (raw file):

Previously, sukovec (Mike S.) wrote…

getter na ID? V tvem examplu nebyl, tak jsem myslel, ze to je umyslne ;)

Done.


src/Entity/SubEvent.php, line 19 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

Viz výše.

Done.


src/Entity/SubEvent.php, line 34 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

type="text"

Done.


Comments from Reviewable

@sukovec
Copy link
Member Author

sukovec commented Feb 12, 2018

Review status: 4 of 7 files reviewed at latest revision, 7 unresolved discussions.


src/Entity/Event.php, line 21 at r2 (raw file):

Previously, michalhosna (Michal Hošna) wrote…

Tedy je otázka jestli psáť propery protected nebo private.

Když pak dědíš tedy entitu rozšířuješ, tak bys nutně rodiče měnit nemusle, měl bys u value objectu jen rozšiřovat. Tzn. není nutné aby bylo protected.

Done.


Comments from Reviewable

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

Successfully merging this pull request may close these issues.

3 participants