HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Form builder class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formclassbuilder

Problem

I am trying to learn OOP in PHP. My PHP knowledge is pretty good, but haven't tried OOP before. I'm currently building a formbuilder class. Can you take a look and tell me if this is the right way?

```
_method = $method;
$this->_action = $action;
}

public function addInput($type,$name,$value,$placeholder = NULL,$extra = NULL) {
$this->_inputs[] = array (
'type' => $type,
'name' => $name,
'value' => $value,
'placeholder' => $placeholder,
'extra' => $extra
);
}

public function addLabel($name) {
$this->_inputs[] = array (
'type' => 'label',
'name' => $name

);
}

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

public function printForm() {
$html = '_action.'" method="'.$this->_method.'">';
for ($i = 0; $i _inputs); $i++) {
switch($this->_inputs[$i]['type']) {

case "label":
$html .= '_inputs[$i]['name'].'">'.$this->_inputs[$i]['name'].'';
break;
case "text":
case "password":
case "submit":
case "email":
$html .= '_inputs[$i]['type'].'" ';
$html .= ' name="'.$this->_inputs[$i]['name'].'" ';
$html .= ' value="'.$this->_inputs[$i]['value'].'" ';
$html.= ' placeholder="'.$this->_inputs[$i]['placeholder'].'"> ';
$html .= ' '.$this->_inputs[$i]['extra'

Solution

I think your code is good overall, I only have a couple of small points:

  • I wouldn't print inside the form class, but return the HTML instead, and then print inside the calling class.



  • I don't like the way you handle labels, as it's very inflexible. I have to use the same for value as the text of the label (what if the text is "5 Cats", which isn't allowed as id?). It's also not clear from the outside that this is how it works (as you are missing PHPDoc comments, and the parameter is called name, not id or for.



  • You are missing some functionality, such as adding classes or ids to the tags. Maybe you can add a generic addAttributeTo($name $key, $value) method, which adds the key/value pair to the element specified by name.



  • I would think about defending XSS in this class, so the caller doesn't have to remember doing it.

Context

StackExchange Code Review Q#83470, answer score: 2

Revisions (0)

No revisions yet.