#1
  1. For POny!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2012
    Location
    Amsterdam
    Posts
    416
    Rep Power
    115

    Small form class question


    Hi guys,

    I just made an attempt to create a sort of form class (not finished yet, will get awesome ) But I just happen to have a tiny problem which is probably caused by my inexperience with oop. The form is being printed but the labels and formfields are not. I have a feeling I can't overwrite the stuff I constructed. Any ideas?

    PHP Code:
    <?php

    /**
     * @desc    form class Builds the form
     */
    class Form {

        public 
    $form;
        public 
    $formFields;
    /**
     * 
     * @param string $action
     * @param string $method
     * @param str $class
     * @param str $id
     */   
        
    public function __construct($action=''$method='post'$class ''$id '') {

            
    $action 'action="' $action '"';
            
    $method 'method="' $method '"';
            
    $class = !empty($class) ? 'class="' $class '"' '';
            
    $id = !empty($id) ? 'id="' $id '"' '';
            
    // This probably is the badd *** because I have a feeling I can't overwrite this
            
    $this->form "<form $action $method $class $id>
                        
    $this->formFields
                    </form>"
    ;
            
            
        }
        
        public function 
    getForm(){
            return 
    $this->form;
        }
    /**
     * @desc  add a form label
     * @param str $for
     * @param str $textLabel
     * @param str $class
     * @param str $id
     */
        
    public function AddFormLabel($for$textLabel$class ''$id '') { 
            
            
    $class = !empty($class) ? 'class="' $class '"' '';
            
    $id = !empty($id) ? 'id="' $id '"' '';
            
            
    $this->formFields .= '<label for="'.$for.'" '.$id.' '.$class.'>'.$textLabel.'</label>';
           
        }
    /**
     * @desc  add a form field
     * @param str $name
     * @param mixed $value
     * @param str $id
     * @param str $class
     * TODO maybe add form type;
     */
        
    public function AddFormField$name$value ''$id ''$class '') {
            
    $this->formFields .= 'tralalaa some input please...';
        }
        public function 
    AddFormFieldsAndLabels($array){
            
    // some awesome function to add multiple fields and types
        
    }
    }

    /**
     * @desc    FormValidation checks all stuf after submitting
     */
    class FormValidation extends Form {
        
    }
    $form = new Form();
    $form->AddFormField('tralala''someidname''someclassname');
    $form->AddFormLabel('someidname''some awesome label''classname''idname');
    echo 
    $form->getForm();
    ?>
    P.s. I have a feeling that the part i commented in the __constructor is causing the problem. I just don't see how to alter its value. Should I maybe remove the stuff in the constructor and place it in a separate function?

    -edit; maybe I should leave out the end tag and the form elements in the constructor? and make a closing the form function?

    edit2: yep that seems to be it, i changed the code to this:
    PHP Code:
        public function __construct($action=''$method='post'$class ''$id '') {

            
    $action 'action="' $action '"';
            
    $method 'method="' $method '"';
            
    $class = !empty($class) ? 'class="' $class '"' '';
            
    $id = !empty($id) ? 'id="' $id '"' '';
            
            
    $this->form "<form $action $method $class $id>";
                       
        }
        
        public function 
    getForm(){
            return 
    $this->form.$this->formFields.'</form>';
        } 
    and it works.

    Still Any critiques tips or hints and improvements are more than welcome.
    Last edited by aeternus; January 2nd, 2013 at 06:17 PM.
  2. #2
  3. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Hi,

    in the previous code, your getForm() method would only return the "form" attribute, which is set in the constructor and never changed. So your later calls to AddFormLabel() and AddFormField() simply didn't have any effect on the rendered form (they only changed the "formField" attribute).

    In any case, you should organize the code in a different way: Instead of directly constructing HTML snippets, you should only store the data (like class, id etc.) and have the form rendered at the very end with a central render() method.

    This has several advantages:
    • You can easily change the HTML markup without having to go through all methods in search for the right string snippet. It's basically the same problem that people face when they're using "old school PHP" with HTML and PHP intermixed. Storing data in attributes and rendering the visible output of the form are two completely separate aspects, so they should be in separate methods.
    • It's possible to change the form data at any time, because it isn't "baked into" strings.
    • It's simply more readable, because each method only does what's it supposed to do.


    Apart from that, I'm generally sceptical towards this kind of "HTML wrappers".

    I think it's a step backwards with regard to separating logic and output (as the MVC pattern teaches us). You'll basically have the same "spagetti code" as people plastering their HTML with "<?php ?>" snippets -- the only difference is that you're doing it object oriented.

    And I doubt the practical use. The PHP code for generating the form won't be much shorter than the actual HTML -- if it all. So it doesn't really make things easier. Actually, the HTML will be much more readable, because it uses a visible structure as opposed to explicit commands "put this element there".

    I think using external template files is still by far the best solution. Maybe some day we'll even have a good template engine, something like Haml for PHP.

    Don't get me wrong: Your approach is certainly interesting and a great way of learning OOP. Just recently, another forum member presented a complete GUI library, which mapped all (or at least the most important) HTML elements to PHP objects. Pretty impressive. But if you asked me if I'd actually use it in real life: probably not. Object oriented language simply aren't good for describing things (like GUIs). That's what declarative languages are for.

    // edit:
    OK, I see this is related to another problem. I think you might have misunderstood what requinix suggested. He talked about a class to hold data related to the form, not a class to actually render the form.
    Last edited by Jacques1; January 3rd, 2013 at 12:18 AM.
  4. #3
  5. For POny!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2012
    Location
    Amsterdam
    Posts
    416
    Rep Power
    115
    Hi Jacques,

    Thanks for your time helping me out. I indeed started working on this class after requinix told me to store data in a sort of form class. But I thought I store everything related to the form even the template For a weird reason I find it easier to understand things when they are sort of real objects Your suggestion with a render method sounds great, maybe I can combine that with a separate template. Although I wanted to dynamically create forms with an unknown amount of fields. So I thought the only thing that al forms have common is the beginning and endtag and the method and action. That's why I put them in the constructer, but indeed screws me over when I am adding fields

    Thanks for your help and time again! I am just a bit inexperienced with this stuff
    Last edited by aeternus; January 3rd, 2013 at 10:32 AM.
  6. #4
  7. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    I'm actually suggesting to not use any rendering logic at all in your PHP code. I don't see how this would solve your problem of storing error messages, and it will leave you with an ugly mixture of programming logic and presentational stuff.

    Using objects to store everything related to a certain aspect is kind of a typical mistake. Language objects do not represent objects of the real world. This might be a good image when you're just starting with OOP (a "Dog" object has a "name" attribute and a "bark" method etc.). But when you're actually writing OOP code, you should get to a more abstract understanding of objects.

    Objects are tools to solve a specific problem. All their attributes and methods solely exist for this problem. If you write a PHP page for a pet shop, a "Dog" object will hardly have a "bark" method to output "Woof! Woof!", because that's simply not relevant for a shop. What it will have is a "price" attribute and maybe an "AddToCart" method.

    So don't try to model a "real" HTML form. Think about what attributes and methods your object needs to store error messages -- probably something like addError($field, $message) and getError($field). The rendering will take place on a different layer, ideally in a separate HTML template file. There you'll fetch the error messages and display them.
  8. #5
  9. For POny!
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2012
    Location
    Amsterdam
    Posts
    416
    Rep Power
    115
    Thanks again Jacques!

    My error message question was sort of related to the above question, but of a different kind (that was focused purely on passing along a globally accessible array). I do although understand what you're saying, and i noticed you didn't suggest the rendering as an ideal situation, in fact you don't encourage it. I just found it a nice little project to work on to practise my oop, pretty much still in the bark and woof woof stage hence my misunderstanding. Although I realize logic and looks should be separated in the mvc model. The thing was I wanted something to create a dynamic form. I don't see how I can do that with purely a template since that implies a bit of static stuff.

    I'll try to look to objects more as tools, although I'll probably make some more mistakes Thanks as always! really appreciate it.
    Last edited by aeternus; January 3rd, 2013 at 09:04 PM.

IMN logo majestic logo threadwatch logo seochat tools logo