OOP PHP form ok?

Question

I am new at OOP and I wonder if the code below is OOP or it can be better and also what can be better?

class cForm{

    private $_TagAction;
    private $_TagMethod;
    private $_TagName;
    private $_TagExtraAttr;

    //var form field
    private $_FieldType;
    private $_FieldName;

    //var button
    private $_ButtonValue;
    private $_ButtonName;

    public function __construct( $_TagAction , $_TagMethod , $_TagName , $_TagExtraAttr ){
        $this->_TagAction = $_TagAction;
        $this->_TagMethod = $_TagMethod;
        $this->_TagName = $_TagName;
        $this->_TagExtraAttr = $_TagExtraAttr;

    }

    public function getTagOpen(){
        return '<form action="' . $this->_TagAction . '" method="' . $this->_TagMethod . '" name="' . $this->_TagName .'" ' . $this->_TagExtraAttr . '>' . PHP_EOL;
    }

    public function setFormField( $FieldType , $FieldName ){
        $this->_FieldType = $FieldType;
        $this->_FieldName = $FieldName;
    }    

    public function getFormField(){
        return '<input type="' . $this->_FieldType . '" name="' . $this->_FieldName . '" >'. PHP_EOL;
    }


    public function setButton( $ButtonValue , $ButtonName ){
        $this->_ButtonValue = $ButtonValue;
        $this->_ButtonName = $ButtonName;
    }

    public function getButton(){
        return '<input type="submit" value="' . $this->_ButtonValue . '" name="'. $this->_ButtonName .'">';
    }

    public function getTagEnd(){
        return '</form>' . PHP_EOL;
    }

}



$objForm = new cForm( '/' , 'post'  , 'test' , 'onsubmit="test"' );
echo $objForm->getTagOpen();

echo $objForm->setFormField('text','2');
echo $objForm->getFormField();

echo $objForm->setButton('test value','test name');
echo $objForm->getButton();

echo $objForm->getTagEnd();

Answer 1

If you have more than one button or input field per form, or are ever using the same instance for more than one form, then no, this code is not OO at all. Even if you only have one, it's quite a stretch, and the way you're using it in your example is not very OO. If you'd set everything first and then echoed everything out, i could see the point. But pretty much all you're doing is splitting the functionality of generating an HTML element into half-procedures, and you're not even taking advantage of the few benefits of doing so.

I was trying to come up with a way to objectify this, but each path amounts to a total rewrite. So instead, let's discuss principles, and how this class might be better written to be more OO.

First off, an object is basically data that does stuff. As in, active data. Your class's approach makes data extremely passive, and amounts to the aforementioned "half-procedures" thing. It's all "store params, generate element, store params, generate element" etc. All you're doing is hiding procedures in objects.

Secondly, your class seems to be violating the heck out of SRP, in that you have it doing no less than three totally separate things (generating form tags, generating buttons, and generating input fields) that are only logically linked by all having something to do with forms. They don't use the same fields, or any of the same methods. They don't even have to be associated with the same form! You basically have three separate classes masquerading as one.

A class should have one clear, well-defined purpose. Like, in this case, to generate a form. I'd personally lose (or make private) the getters for the individual elements, and have one method that generates a form complete with buttons and inputs and opening and closing tags.

Alternatively, you could break the class up into three...but that leaves the caller having to arrange form elements itself. You're not really gaining anything from using objects if the caller has to micromanage all the interactions. OOP is all about the objects doing most of those interactions on their own, and the caller just firing off requests to do sets of things. The more you can get objects to do independently, the more leverage you have, and the more you can get done with one method call.

Oh, and namingwise:

• It's generally expected that when you setSomething and then getSomething, you get back the same thing you set, or at least something similar.
• Hungarian notation is dead. Let it rest in peace, by losing the c in cForm.

Answer 2

If you are building an Object-Oriented Form then you need to start with building the HTML elements. Once you have that, you can easily build from out of them.

class Form extends HtmlElement {
function init(){
    parent::init();
    $this->setElement('form');
}
function addInput($type,$name,$value,$tag='Content'){
    $f=$this->add('HtmlElement',$name,$tag);
    $f->setElement('input');
    $f->setAttr('type',$type);
    $f->setAttr('value',$value);
    $f->set('');
    return $f;
}
}

This example illustrates two very significant principles of OOP: inheritance (you inherit form from HtmlElement, thus it retains all the properties such as setAttr, setElement, and also setting contents) and the second principle is encapsulation - your Form consists of many other objects.

You can also base HtmlElement on DOM objects, but in my case, here is the implementation of HtmlElement:

Answer 3

A very rough view I have about improving this is having a method in the class to create a form, not the class being a form, leading to a separate instance for each form; Just this is just a brief extension of what cHao mentioned at the beginning.

class cForm
{
  public function __construct()
  {
    //let it set some global html values related to the charset or something.
  }

  public function createForm()
  {
     //The old construct
  }

 public function generateFormElement($formAttributes/*, optional parameters*/)
 {
   if(is_array($formAttributes))
  {
   //break down the array and generate form elements, add checks to suit each type.
  }else if(isset(/*optional variables, i.e. form type, etc.*/ $optional))
   {
   //create the form from provided information
   }
  }
}

Usability

• Next on, you're not adding anything special to creation of forms using this class, - personally, I'd enjoy writing pure html than using functions that might make my work harder and not worthy.

For example, have you taken a look at the form generator of codeigniter or even zend? They provide security according to the field type or specific parameters provided to the function.

• Have you noticed that all those functions could be swallowed down to one or 2 functions? In the end, you're creating input tags, all you need is to improvise and add in some clever ways to set different types of forms in different situations needed e.g. generateFormElement function.

And you have the simplest possible flexible module that might suit custom use.

Flexibility

Also, I see a complete lack on the utilization of arrays. Don't you think that using an array to create multiple form elements is much more flexible that calling a function whenever I need to create a new element?

Exceptions

People now-a-days seem to underestimate the power of exceptions, you are not handling input to the function what so ever, and not producing any exceptions. A beautiful class is that which allows use of custom error messages(pure opinion).