Formbuilder -> beoordelen

je class heeft teveel verantwoordelijkheden!

Maak alles wat abstracter... dit kan je doen door bv een aantal elementen in een aparte class te zetten.

Dingen met $this->errors[] moet je met een validator doen, niet alles in 1 classe...

Dan nog iets, dingen als dit:

$this->errors[] = 'Er is een error in de formbuilder. Het type '.$type .' bestaat niet in de categorie input fields.';

Zou je met een exception moeten doen.

Verder nog commentaren? Wil er zoveel mogelijk van opsteken dus gooi alles er maar in :)

En Raoul, ik vind exceptions altijd heel vervelend om te lezen. Daarnaast kan een exception maar 1 error bevatten. Deze methode meerdere, is dat niet handiger?

Dit is geen OOP. Dat maakt niks uit, want het duurt een tijd voordat je het OO concept onder de knie hebt.

Wat je hier hebt gedaan is gewoon 1 klasse gemaakt en die gebruikt om alle functies te bundelen. Een leuke manier van programmeren en totaal niet fout, maar geen OO.

In OO gaat het erom dat je zoveel mogelijk alle verantwoordelijkheden spreid over objecten. Een van de belangrijkste principes is het Single Responsibility Principle: Een object mag maar 1 rede hebben om te veranderen. In jouw object zijn er meerdere redenen om hem te willen veranderen:
- als we de HTML veranderen (de "presentatie" laag)
- als we een nieuw field type willen toevoegen
- als we ipv $_POST bijv. $_GET willen gebruiken (gebruik nooit superglobals in OO objecten)
- als we het AJAX script willen veranderen (of helemaal geen AJAX willen gebruiken)

Al deze redenen krijgen een eigen klasse. Je krijgt dan verschillende FormType klassen, een FormRenderer/FormView klasse, een FormRequest klasse, een FormBuilder klasse, een Form klasse, etc.

Dankje wouter :)

Oke, nou heb ik 2 classes: een FormRenderer en een FormView class, hoe kan ik die samen laten werken?

Wouter vat zowat alles samen. Je moet gewoon beter het OO principe onder de knie krijgen, maar met dingen als dit leer je het :-)

Dus: alles verantwoordelijkheden verspreiden in classes en abstracter maken.

[size=xsmall]Toevoeging op 08/04/2014 15:22:15:[/size]

Donny Wie weet op 08/04/2014 15:21:24

Dankje wouter :)

Oke, nou heb ik 2 classes: een FormRenderer en een FormView class, hoe kan ik die samen laten werken?

Wat is het verschil tussen FormRenderer en FormView? Doen ze niet allebei hetzelfde?

Sorry, bedoelde FormRequest en een FormRenderer

[size=xsmall]Toevoeging op 08/04/2014 15:31:38:[/size]

Zou ik bijvoorbeeld dit mogen doen:

if(server request == post){
formvalidator aanroepen
} else {
formrenderer aanroepen
}

of moet alles juist samen werken?

Dat lijkt weer meer op procedural dan oop en dat wordt hierboven niet bedoelt.
Geen if-else, maar meerdere klasses die hun eigen verantwoordelijk hebben.
Kijk eens naar andere OOP scripts, bijv een twitter api

>> Geen if-else, maar meerdere klasses die hun eigen verantwoordelijk hebben.

OOp mag ook IF elsejes hebben... En daarnaast hebben we hier meerdere klassen met hun eigen verantwoordelijkheid...
Als laatst zou ik jouw twitter api voorbeeld ook geen OO willen noemen...

Donny, ja zoiets.

Lees zeker over de verschillende soorten injection.

Dus, zo'n opzet lijkt me wel correct:

<?php

$validator = new FormValidator();
$validator->addRule('username', 'required');

$form = new LoginForm();

$renderer = new FormRenderer($form); // constructor injection

if ($_SERVER['REQUEST_METHOD'] == 'POST')
{
	if ($formValidator->valid($form))
	{
		// OK
	}
	else
	{
		// doe iets met $formValidator->getMessages()
	}
}

echo $renderer->getOutput();
?>

[size=xsmall]Toevoeging op 08/04/2014 19:42:49:[/size]

En die irritante code tag bug in de UBB is weer terug.. *Zucht*