php form builder class, render in Twig

[mjjdesignworks]

Firstly the code:

 

Formbuilder.php

class FormBuilder
{
protected $input;
private $method;
private $action;

public function __construct($method, $action = NULL)
{
$this->method = $method;
$this->action = $action;
}

public function newInput($type, $name, $value, $placeholder = NULL, $class = NULL, $id = NULL)
{
$this->input[] = array (
'type' => $type,
'name' => $name,
'value' => $value,
'placeholder' => $placeholder,
'class' => $class,
'id' => $id
);
}

public function newLabel($name, $title = NULL)
{
$this->input[] = array (
'type' => 'label',
'name' => $name,
'title' => $title

);
}

public function newTextarea($name, $value = NULL, $placeholder = NULL)
{
$this->input[] = array (
'type' => 'textarea',
'name' => $name,
'value' => $value,
'placeholder' => $placeholder
);
}

public function renderForm()
{

$html = '<form action="' . $this->action . '" method="' . $this->method . '">';
for ($i = 0; $i < count($this->input); $i++) {
switch($this->input[$i]['type']) {

case "label":
$html .= '<div><p><label for="' . $this->input[$i]['name'] . '">' . $this->input[$i]['title'] . '</label><br />';
break;
case "text":
case "password":
case "hidden":
case "submit":
case "email":
$html .= '<input type="' . $this->input[$i]['type'] . '" ';
$html .= ' name="' . $this->input[$i]['name'] . '" ';
$html .= ' value="' . $this->input[$i]['value'] . '" ';
$html .= ' placeholder="' . $this->input[$i]['placeholder'] . '" ';
$html .= ' class="' . $this->input[$i]['class'] . '" ';
$html .= ' id="' . $this->input[$i]['id'] . '"></p></div> ';
break;
case "textarea":
$html .= '<textarea name="' . $this->input[$i]['name'] . ' value="' . $this->input[$i]['value'] . '" placeholder="' . $this->input[$i]['placeholder'] . '"></textarea>';
break;
}

}
$html .= '</form>';
$form = sprintf($html);
// print $form;

Registration.php

class Registration extends \Core\CoreController
{
public function buildForm()
{
$regform = new FormBuilder('post');
$regform->newLabel('username', 'Enter a Username:');
$regform->newInput('text','username','','Enter Username', 'form-control', 'username');

$regform->newLabel('password', 'Enter password:');
$regform->newInput('password', 'password', '', 'Enter Password', 'form-control', 'password');
$regform->newLabel('password', 'Re-enter password:');
$regform->newInput('password', 'password_chk', '', 'Password Again', 'form-control', 'password_chk');

$regform->newLabel('useremail', 'Enter email address:');
$regform->newInput('email','useremail','','Email Address', 'form-control', 'useremail');

$regform->newInput('submit','submit','Register');

$regform->renderForm();
}

public function registerAction()
{
$newform = $this->buildForm();
View::renderTwigTemplate('Frontend/Tests/tests.html.twig',[
'newform' => $newform
]);
}
}

}
}

Twig Template

 

%

extends "base.html.twig" %}

{% block body %}
{{ newform }}
{% endblock %

The problem:

 

As you can see in Formbuilder.php i assign a variable ($form) to a sprintf function, i have to use sprintf because if i use print it overrides the twig template and just prints the form even without being called by Twig at least with sprintf i can assign it to a variable.

 

What i am trying to do for hours now is get my $form variable inside the buildForm method in Registration.php where i can then assign it to a Twig variable and render it where i want it inside the Twig template, as i said using print just does what its supposed to do, print the form above my call to the extends "Base.html.twig" which is obviously not what i want.

 

I could possibly get away without having to assign a variable to sprintf function as this line of code ($regform->renderForm) in Registration.php buildForm method is what renders the print so im guessing it also holds the sprintf, i just need to find a way to assign it to the $newform variable for Twig.

 

Thanks

[kicken]

You need to return your HTML string from your renderForm function, not try to print it. Your use of sprintf does nothing, that line is basically the same as just doing $form = $html which is pointless.

 

Once you return the HTML from renderForm you also need to return it from your buildForm method. That will then let you assign it to your $newform variable and pass it to twig.

[Jacques1]

The whole code has no security whatsoever, which is particularly disastrous when you're writing an abstraction layer like this form builder. As soon as you (or worse: somebody else) use the builder with dynamic parameters, you end up with bugs and XSS vulnerabilities all over the place, and it's not even clear where on earth they come from, because all the markup is hidden within the class.

 

The class design isn't very good either. Long switch statements are in warning sign in OOP, and indeed your class is just one monolithic block of procedural code. The only realistic way to modify or extend the functionalities is to edit the source code – which is the opposite of OOP. Even worse, you have hard-coded design elements (<p>, <br>, <div>) everywhere, and they depend on each other, which means the class really isn't a form builder, at best it's a shortcut for your own (rather quirky) form fragments. It's not useful for anything or anybody else.

 

Besides that, I still haven't figured out why PHP people love to create their own “form builders” so much. This piece of code

$regform->newLabel('password', 'Enter password:');
$regform->newInput('password', 'password', '', 'Enter Password', 'form-control', 'password');

could simply be written as

<label>Enter password: <input id="password" class="form-control" type="password" name="password" placeholder="Enter Password"></label>

This has less characters and is arguebly a lot more readable, so how exactly is the form builder useful?

 

Personally, I would drop the class and use good(!) old HTML instead. Note that Twig has macros, so you can always define shortcuts for HTML fragments.

 

If you absolutely must use a form builder, you should either use a proper library or actually spend a lot of time on the class design (and security). A good class has to do more than barely “work”.

[mjjdesignworks]

ironically enough i have just done exactly that around an hour ago, just a few small changes were required:

 

this at the top of the Formbuilder class:

class FormBuilder
{
public $form;

Still in Formbuilder (renderForm method)

 

changed this:

$html .= '</form>';
$form = sprintf($html);

to this:

$html .= '</form>';
$this->form = sprintf($html)

and then carried the new $form variable over to the Registration class (buildForm method)

and changed this

$regform->renderForm();

}

public function registerAction()
{
$newform = $this->buildForm();
View::renderTwigTemplate('Frontend/Tests/tests.html.twig',[
'newform' => $newform
]);
};

to this

$regform->renderForm();

$newform = $regform->form;
return $newform;
}

public function registerAction()
{
$newform = Registration::buildForm();
View::renderTwigTemplate('Shared/Userforms/register.html.twig',[
'newform' => $newform
]);
}
}

Only issue i had was with Twig auto-escaping the ouput and the form rendering as on page html which meant having to declare autoescaping to "false" in the Twig template file.

[mjjdesignworks]

As Jacques1 points out there is no security and right now this isn't an issue, these are my first steps into learning oop and it is just a work in progress, criticism's are welcome and noted, the objective from my point of view was to get it working and to see how it flows, it will be tidied up (maybe even rewritten) and improved with xss and csrf protection and proper form validation.

[kicken]

just a few small changes were required:

Those changes were not needed. The only changes you needed to make to pass the HTML back is:

 

Change

$form = sprintf($html);

to

return $html;

and change

$regform->renderForm();

to

return $regform->renderForm();

[Jacques1]

How about we start worrying about the real issues? If the OP walks away with that code and tells everybody that kicken from PHP Freaks has OK'd it, that would be pretty embarassing, don't you think?