PHP, OOP - Form Validation

Question

I have created a form element class. The class is able to

Create HTML form elements and
Confirm if form has been submitted and if so, load the submitted value, for the specified form element into the object which is worked on on the page.

I have tried to make this as general as possible in order to make it reusable but since I am kind of newbie with regards to OOP it would be super cool if someone could check this and let me know if this is good OOP also if this is a good solution for what I am trying to achieve.

Here is the essential part of the class_FormControl()

class FormControl{
    var $finalcontrol;
    var $class = "form-control";
    var $form_error;

    protected function StartFormatting($name, $label){
        if (isset($_POST[$name]) AND $_POST[$name] != "") {
            return false;
        }
        $this->finalcontrol = "<label for='$name' >$label</label>";
        return true;
    }

    public function get_control(){
        return $this->finalcontrol;
}
}

class TextBox extends FormControl{

    public function CreateControl($obj, $name, $label, $placeholder, $value = ""){
        if($this->StartFormatting($name, $label)){
            $this->finalcontrol .= "<input type='text' class='$this->class' id='$name' name='$name' placeholder='$placeholder'";

            if ($value != "") { 
                $this->finalcontrol .= " value='$value' "; 
            }

            $this->finalcontrol .= ">";
            return true;
        } 

        $func = "set_" . $name;
        $obj->$func($_POST[$name]);
        return false;

    }
}

And here is how I use the class in the form page:

$r1 = New Recipe();
$tbx = new TextBox();
$ctrl1 = $tbx->CreateControl($r1, "Name", "Nombre", "Nombre", $r1->get_Name());

Now if $ctrl1 is true I go on and save the object in the database.

If $ctrl1 is false I go on and

echo $tbx->get_control();

on the right place in the page.

/Thanks!

@PeeHaa - Yeah of course, to display one textfield this is overkill. But I thought having hundreds of fields on all the subpages, also including saving values to the object inside the class Ithough it would pay off in the long run... I guess your commen is answer No to question 2 then. Noted. — user3720199
– user3720199, Commented Jun 21, 2014 at 21:28
I see no advantage using a function than standard HTML. If you need a thousand of textfields, you can use a loop. — Ivan Yonkov
– Ivan Yonkov, Commented Jun 21, 2014 at 21:31
And, don't want to sound discouraging, but your OOP does not rely on any object principle, despite the fact you have base and child class, all this thing is pretty much functional programming. And as well, in PHP5 the properties has access levels too, also according to the convention around PHP community, methods should be camelCase. — Ivan Yonkov
– Ivan Yonkov, Commented Jun 21, 2014 at 21:35
Not discouraging at all :) I am very grateful for your feedback, and spending just a few minutes reading I see what you mean. I will go back studying more before I continue with this. Thanks! — user3720199
– user3720199, Commented Jun 21, 2014 at 21:57

Jimbo · Accepted Answer · 2014-07-22 12:04:22Z

Note: This is not the only way to do it; it is one of many and it is my personal interpretation of the problem in 10 minutes on my lunch break. Please bear in mind that this would be my implementation with the little information I'd have been given. In the real world, I would find out more about the domain, but I would still adhere to the single responsibility principle in my objects and keep everything loosely coupled. These are the main things to take away from this post.

OOP

First and foremost, you need to be thinking in terms of objects. Each object has it's own single responsibility wikipedia entry:

In object-oriented programming, the single responsibility principle states that every context (class, function, variable, etc.) should have a single responsibility, and that responsibility should be entirely encapsulated by the context. All its services should be narrowly aligned with that responsibility. [emphasis my own].

What you are doing is placing procedural code into class methods. This does not make it object oriented! You need to shift your mindset!

Objects

You're building a form builder. A form is comprised of elements. Instantly, I am thinking of:

A Form object
A FormElement object

The above objects are specific representations of an Entity or ValueObject. Domain Driven Design.

FormElement could be an Interface that all form elements (like input boxes, buttons etc) must conform to.

class Form
{
   /**
    * @var FormElement[]
    */
    protected $elements;

   /**
    * Add a FormElement to the form
    *
    * @param FormElement $element
    */
    public function addFormElement(FormElement $element)
    {
        $this->elements[] = $element;
    }
}

interface FormElement
{
   /**
    * @return The generated html for the given form element
    */
    public function getHtml();
}

Now all you need to do is make sure that each of your objects that implement FormElement return exactly what you want within the FormElement::getHtml() method and it'll all still work great when you add new elements to the Form, because it will be the Form that is calling getHtml() in a loop on each of the FormElement objects before adding this to it's own HTML and outputting it.

Here's an example of a TextBoxFormElement I would use:

class TextBoxFormElement implements FormElement
{ 
   /**
    * @constructor
    *
    * This is where I am declaring that, for this to be a VALID object, it MUST have
    * the following properties passed in
    *
    * @param string $name
    * @param string $label
    * @param string $placeholder
    * @param string $value
    */
    public function __construct($id, $class, $name, $label, $placeholder, $value)
    {
        $this->id          = $id;
        $this->class       = $class;
        $this->name        = $name;
        $this->label       = $label;
        $this->placeholder = $placeholder;
        $this->value       = $value;
    }

   /**
    * Generate the html of this element
    *
    * @return string
    */
    public function getHtml()
    {
        return sprintf(
            "<input type='text' id='%s' class='%s' name='%s' label='%s' placeholder='%s' value='%s'>",
            $this->id, $this->class, $this->name, $this->label, $this->placeholder, $this->value
        );
    }
}

SuperGlobals

You are using $_POST and $_GET in these classes. You should not be doing this. You should be doing the following:

Each object your write, the public methods of these objects dictate their API. You are writing an API to those object's usages. [My own quote]

Effectively, having any sort of $_POST or $_GET couples these objects to the state of these superglobals. How are you going to test them? You're going to have to mock out (or fake) the contents of these superglobals each time you want to test them.

No. What you should be doing is forgetting about your own application for a moment and coding these objects so you can use them in any application. Then, you pass in the values into the objects constructor or methods. Do not use $_POST or $_GET directly within this class.

Conclusion

You are not writing OO code. But I'm not going to write it for you. I will, however, write the methods that I would expect to call when using your library in the hope that you can figure out for yourself your own implementation. Keep everything separate, use a single responsibility for every object (this is not overkill, it is object oriented programming), and keep learning:

// Sanitize and validate input data obviously you would have a RequestValidator used before you even get to this point, making sure exactly what you want can be used by the library
$name = $_POST['name'];

/** Create our form **/
$form = new Form;

/** TextBoxFormElement implements FormElement **/
$textBox1 = new TextBoxFormElement($name, 'Label', 'Placeholder' /** etc **/);

/** Add our FormElement **/
$form->addFormElement($textBox1);

/** Generate our html - the form generates it's own HTML as well as calling getHTML() on the FormElement objects in a loop **/
echo $form->getHtml();

That is IT. That is the object API you should be creating. It should be as simple as that. Now go forth with this magical knowledge and learn.

Extra info: An alternative is to take a look at the Visitor Pattern. Basically, you separate the thing that generates the output from the thing that holds the data. Here's more info on the topic if you want to take a look at that.

Always remember that your code should be readable and easy to understand, and that if it helps - you should create some industry standard UML diagrams to go alongside your code.

This is the best example, I have come across, for using interfaces.

Collectives™ on Stack Overflow

PHP, OOP - Form Validation

1 Answer 1

OOP

Objects

SuperGlobals

Conclusion

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

OOP

Objects

SuperGlobals

Conclusion

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related