SitePoint Sponsor

User Tag List

Results 1 to 19 of 19
  1. #1
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    SimpleTest...have I got the right idea?

    I was wondering if somebody could let me know if I've got the right idea with the following test, and perhaps offer some pointers. Please note that I am writing this test for a class that was previously written.

    The Form class:

    PHP Code:
    <?php
    /**
     *******************************************************
     * wildcat.forms.Form
     * Base form class
     *******************************************************
     * @author Luke Redpath
     * @version 1.0
     * @copyright Copyright &copy; Luke Redpath 2005
     *******************************************************
     */
     
    if (!defined('WILDCAT_PATH')) {
        
    define('WILDCAT_PATH''../../classes/');
    }

    /**
     * Form
     * @package wildcat.forms

     */
    class Form
    {
        
    //    MEMBERS
        
    protected $_id;
        protected 
    $_method;
        protected 
    $_action;
        protected 
    $_fieldsets;
        protected 
    $_isValid;
        protected 
    $_formErrors;
        private 
    $_successView;
        private 
    $_failedView;
        
        
    //    CONSTRUCTOR
        
    function __construct($id$action$method)
        {
            
    $this->_id $id;
            
    $this->_method $method;
            
    $this->_action $action;
            
    $this->_fieldsets = array();
            
    $this->_formErrors = array();
        }
        
        
    //    MANIPULATORS
        
    function createFieldset($id$legend='')
        {
            require_once(
    WILDCAT_PATH.'forms/Fieldset.class.php');
            
            
    $fs = new Fieldset($this->_id.'_'.$id$legend);
            
    $this->_fieldsets[] = $fs;
            return 
    $fs;
        }
        
        function 
    display()
        {
            
    // check for fieldsets
            
    if(count($this->_fieldsets)==0) {
                throw new 
    Exception('Form error: form ['.$this->_id .'] contains no fieldsets.');
            }
            
            
    $form_html '<form id="'.$this->_id.'" method="'.$this->_method.'" action="'.$this->_action.'">';
            
            foreach(
    $this->_fieldsets as $fieldset) {
                
    $form_html .= $fieldset->render();
            }
        
            
    $form_html .= '</form>';
        
            return 
    $form_html;
        }
        
        function 
    isSubmitted()
        {
            if(isset(
    $_REQUEST['submit_'.$this->_id])) {
                
    // form is submitted so populate the form
                // with submitted values and validate
                
    $this->repopulateForm();
                
    $this->validate();
                return 
    true;
            } else {
                return 
    false;
            }
        }
        
        function 
    isValid()
        {
            return 
    $this->_isValid;
        }
        
        private function 
    validate()
        {
            
    $this->_isValid true;
            
            
    // loop through fields and check they are valid
            
    foreach($this->_fieldsets as $fieldset) {
                foreach(
    $fieldset->getFields() as $field) {
                    if(!
    $field->isValid()) {
                        
    $this->_formErrors[] = array('fieldLabel' => $field->getLabel(),
                                                     
    'errMsg' => $field->getError());
                        
    $field->setValue('');
                    }
                }
            }
            
            if(!empty(
    $this->_formErrors)) {
                
    $this->_isValid false;
            }        
        }
        
        private function 
    repopulateForm()
        {
            foreach(
    $this->_fieldsets as $fieldset) {
                foreach(
    $fieldset->getFields() as $field) {
                    
    $field->setValue($_REQUEST[$field->getName()]);
                }
            }
        }
        
        function 
    displayFormData()
        {
            
    $dataHTML '';
            
    $data $this->getAllDataFields();
            foreach(
    $data as $field) {
                
    $dataHTML .= '<strong>'.$field->getLabel().'</strong>: '.$field->getValue().'<br />';
            }
            return 
    $dataHTML;
        }
        
        function 
    getAllDataFields()
        {
            
    $tmpFieldsArray = Array();
            foreach(
    $this->_fieldsets as $fieldset) {
                
    $tmpFieldsArray array_merge($tmpFieldsArray$fieldset->getFields());
            }
            
            
    // remove the submit button
            
    unset($tmpFieldsArray['submit_'.$this->_id]);
            
            return 
    $tmpFieldsArray;
        }
        
        function 
    process($processor)
        {
            
    $processor->run($this->getAllDataFields());
        }
        
        
    //    ACCESSORS
        
    function getErrors()
        {
            return 
    $this->_formErrors;
        }
        
        function 
    setSuccessView(FormView $view)
        {
            
    $this->_successView $view;
        }
        
        function 
    setFailedView(FormView $view)
        {
            
    $this->_failedView $view;
        }
        
        function 
    getSuccessView()
        {
            return 
    $this->_successView;
        }
        
        function 
    getFailedView()
        {
            return 
    $this->_failedView;
        }
        
        function 
    getFormId()
        {
            return 
    $this->_id;
        }
    }
    ?>
    The tests:

    PHP Code:
    <?php
    /**
     *******************************************************
     * tests.wildcat.Form
     * Form test
     *******************************************************
     * @author Luke Redpath
     * @version 1.0
     * @copyright Copyright &copy; Luke Redpath 2005
     *******************************************************
     */

    require_once('../TestBase.inc.php');
    require_once(
    SIMPLE_TEST.'simpletest/unit_tester.php');
    require_once(
    SIMPLE_TEST.'simpletest/reporter.php');
    require_once(
    '../../classes/forms/Form.class.php');

    class 
    test_Form extends UnitTestCase
    {
        function 
    __construct()
        {
            
    $this->UnitTestCase('wildcat.forms.Form Test');
        }
        
        function 
    createSampleForm()
        {
            
    $form = new Form('testform''index.php''post');
            
    $fs1 $form->createFieldset('fs1''Some Fieldset');
            
    $fs1->addField('textfield''Text Field''text'true);
            return 
    $form;
        }
        
        function 
    submitSampleForm($form$valid true)
        {
            if(
    $valid) {
                
    // create some valid form data
                
    $_REQUEST['textfield'] = 'Sample Data';
            } else {
                
    // create some invalid form data
                // (in this case an empty required field)
                
    $_REQUEST['textfield'] = '';
            }
                
            
    $_REQUEST['submit_'.$form->getFormId()] = 'true';
        }
        
        function 
    clearRequest($formId)
        {
            unset(
    $_REQUEST['textfield']);
            unset(
    $_REQUEST['submit_'.$formId]);
        }
        
        function 
    testCreatingForm()
        {
            
    $form = new Form('testform''index.php''post');
            
    $this->assertFalse($form->isSubmitted());
        }
        
        function 
    testDisplayingForm()
        {
            
    // create an empty form
            
    $form = new Form('testform''index.php''post');
            
            
    // display() should throw an exception here
            
    try {
                
    $this->assertTrue(is_string($form->display()));   
            } catch(
    Exception $e) {
                
    $this->pass();
            }
            
            
    // create a fieldset
            
    $fs1 $form->createFieldset('fs1''Some Fieldset');
            
            
    // display() should still throw an exception
            
    try {
                
    $this->assertTrue(is_string($form->display()));   
            } catch(
    Exception $e) {
                
    $this->pass();
            }
            
            
    // add a field to the fieldset
            
    $fs1->addField('textfield''Text Field''text'true);
            
            
    // display() should return the form HTML here
            
    try {
                
    $this->assertTrue(is_string($form->display()));   
            } catch(
    Exception $e) {
                
    $this->fail('Form threw an exception, expected the form HTML');
            }
        }
        
        function 
    testSubmittingForm()
        {
            
    $form $this->createSampleForm();
            
    $this->assertFalse($form->isSubmitted());
            
            
    // submit the form
            
    $this->submitSampleForm($form);
            
    $this->assertTrue($form->isSubmitted());
            
            
    // clear the request for other tests
            
    $this->clearRequest($form->getFormId());
        }
        
        function 
    testValidatingForm()
        {
            
    $form $this->createSampleForm();
            
            
    // should be invalid as it hasn't yet been submitted
            
    $this->assertFalse($form->isValid());
            
            
    // submit with invalid data
            
    $this->submitSampleForm($formfalse);
            
    $this->assertTrue($form->isSubmitted());
            
    $this->assertFalse($form->isValid());
            
    $this->clearRequest($form->getFormId());
            
            
    // submit some valid data
            
    $form2 $this->createSampleForm();
            
    $this->submitSampleForm($form2);
            
    $this->assertTrue($form2->isSubmitted());
            
    $this->assertTrue($form2->isValid());
        }
    }

    $test = new test_Form();
    $test->run(new HtmlReporter());
    ?>
    Thanks a lot.

  2. #2
    SitePoint Addict been's Avatar
    Join Date
    May 2002
    Location
    Gent, Belgium
    Posts
    284
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quickly going over it, it looks, at least to me, you have the right idea, but I'd start thinking about refactoring test_Form.
    If you keep the test like this, you'll end up with one big, fat, monolythic class. As always, keep classes small and focused, it's not because they're tests we have to ignore that piece of excellent advice

    I think there's a base FormTestCase class in there, one with an overridable 'createSampleForm()' method for example. (btw, why not use $form = $this->createSampleForm() in testCreatingForm() and testDisplayingForm() ?)

    testValidatingForm(), I would split up in testValidForm() and testInvalidForm() (belonging to a seperate FormValidationTest class): If testValidatingForm() fails now, you'd only know if a true or false assertion failed, but you wouldn't directly know which one (there's 3 true assertions and 2 false assertion in that test method). You could pass an optional message to the assert*() call, but I'd go for the class and method split up.

    Writing tests for previously written components is how I got into unit testing myself, but there's something fishy about it: You tend to write your tests following the design that's already imposed by the component you're testing.
    When writing tests first, you're forced to think about things like api, naming, usage, etc... In your mind you're actually designing the component, I've got a nagging feeling this doesn't happen when you write tests for already existing code; you tend to keep adding tests and they just run green I don't like that, tests are supposed to run red the very first time they're run, seems I'm getting addicted to the red bar instead of the green one
    Per
    Everything
    works on a PowerPoint slide

  3. #3
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks for your comments. I've not gone as far as creating a base FormTestCase form, but I've done a little bit of refactoring based on what you've said and my own observations...

    PHP Code:
    class test_Form extends UnitTestCase
    {
        private 
    $_formObject;
        
        function 
    __construct()
        {
            
    $this->UnitTestCase('wildcat.forms.Form Test');
        }
        
        function 
    setUp()
        {
            
    $this->_formObject $this->createSampleForm();
        }
        
        function 
    tearDown()
        {
            
    $this->clearRequest($this->_formObject->getFormId());
            unset(
    $this->_formObject);
        }
        
        function 
    createSampleForm()
        {
            
    $form = new Form('testform''index.php''post');
            
    $fs1 $form->createFieldset('fs1''Some Fieldset');
            
    $fs1->addField('textfield''Text Field''text'true);
            return 
    $form;
        }
        
        function 
    submitSampleForm($form$valid true)
        {
            if(
    $valid) {
                
    // create some valid form data
                
    $_REQUEST['textfield'] = 'Sample Data';
            } else {
                
    // create some invalid form data
                // (in this case an empty required field)
                
    $_REQUEST['textfield'] = '';
            }
                
            
    $_REQUEST['submit_'.$form->getFormId()] = 'true';
        }
        
        function 
    clearRequest($formId)
        {
            unset(
    $_REQUEST['textfield']);
            unset(
    $_REQUEST['submit_'.$formId]);
        }
        
        function 
    testCreatingForm()
        {
            
    $this->assertFalse($this->_formObject->isSubmitted());
        }
        
        function 
    testDisplayingForm()
        {
            
    // display() should return the form HTML here
            
    try {
                
    $this->assertTrue(is_string($this->_formObject->display()));   
            } catch(
    Exception $e) {
                
    $this->fail('Form threw an exception, expected the form HTML');
            }
        }
        
        function 
    testSubmittingForm()
        {
            
    $this->submitSampleForm($this->_formObject);
            
    $this->assertTrue($this->_formObject->isSubmitted());        
        }
        
        function 
    testValidForm()
        {
            
    // submit with valid data
            
    $this->submitSampleForm($this->_formObject);
            
    $this->assertFalse($this->_formObject->isValid());
        }
        
        function 
    testInvalidForm()
        {
            
    // submit with invalid data
            
    $this->submitSampleForm($this->_formObjectfalse);
            
    $this->assertFalse($this->_formObject->isValid());
        }


  4. #4
    SitePoint Addict been's Avatar
    Join Date
    May 2002
    Location
    Gent, Belgium
    Posts
    284
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Looking better (imho! ).
    Still a bit worried about the 1 big class thing though; it's testing submission, validation and displaying of forms, I really think this is too much responsibility (again, imho).

    I would also be looking for a way to get rid of the $_REQUEST dependency in the form class and just pass it as a parameter. It brings the benefit of being able to throw all kinds of test data at the class, without resorting to setting and unsetting the $_REQUEST variable.
    Per
    Everything
    works on a PowerPoint slide

  5. #5
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Still a bit worried about the 1 big class thing though; it's testing submission, validation and displaying of forms
    This is what confuses me. I thought the purpose of this file was to test the Form class; that is, test what the Form class does. To me, a form can be submitted, validated**, and displayed, therefore thats what the Form class does and thats what this tests. This is after all, just a unit test, and the Form class is a unit. I got the impression from the SimpleTest website that in most cases, its one test case per class.

    The tests I have here is pretty much it for testing the Form class. There are lots of other form-related classes (Fieldset, Field, FieldFactory, FormView, FormController to name a few) that will all be tested in their own test cases.

    **(by validated, I mean the form validates itself as whole by asking, does each of its fields pass validation. The actual task of validating each field is delegated to the Field class (or an extension of the Field class)).

  6. #6
    SitePoint Zealot
    Join Date
    Oct 2004
    Location
    naperville
    Posts
    189
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Maybe its a hint that the one form class is doing too much? One Class, One Task...

  7. #7
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Super Phil
    Maybe its a hint that the one form class is doing too much? One Class, One Task...
    As I explained above, I don't think the Form class is doing too much. It does the 3 main jobs of the form.

  8. #8
    SitePoint Guru
    Join Date
    Oct 2001
    Posts
    656
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It does the 3 main jobs of the form.
    ...
    To me, a form can be submitted, validated**, and displayed...
    That's right. The concept of a form can be submitted, validated and displayed. But that does not mean that there has to be one class responsible for all of this.

    Perhaps this analogy helps: your entire PHP program is responsible for editing content, managing users and displaying access statistics (just an example). But that doesn't mean that you have to put all of that functionality in one class.

    When you separate, for example, the display and validation of a form, you will find that each class has a much more clear and concise role and that it is easier to test. You can easily test whether the class that is responsible for validating a form works correctly, without having to test the display of the form at the same time.

    I don't know how to explain this any better at the moment, but I have a very strong feeling that "a Form class" should be separated into at least a validation and a display component. I think a lot of people here agree with me. Not that you should base your own opinion on that, but just think about the arguments people here give you for splitting up your form class.

    It does the 3 main jobs of the form.
    I am, like many, a believer of the statement 'one class, one role/job'. Your class is doing three things at a time.

  9. #9
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    For the purpose of Unit Testing, I'm of the view that you don't actually need to display a form.

    You just need to validate the submitted data, as at the end of the day, is it really important (for testing purposes only) to display the data?

    Why not just do a dump of the validated data instead, or is there something I've missed...

  10. #10
    simple tester McGruff's Avatar
    Join Date
    Sep 2003
    Location
    Glasgow
    Posts
    1,690
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'd strongly agree with the points made about the scope of your forms class - and the comments about easier testing of leaner classes.

    Object oriented programming is all about cutting the cake and the essential foundation for these decisions is the presentation / domain / data access layering. A lot of things start to fall into place when you consider object roles in these terms. That's just the start though: there's yet more abstract OOP philosophising to be done in breaking down these layers (eg the VC division of the presentation layer in MVC).

    Display is presentation layer and validation is domain layer. These roles really shouldn't be covered by the same class. Classes should be cohesive.

    I'd see a validation specification somewhere ie define a bunch of rules which you can feed into generic validation code. A controller will interpret the validation results to decide what to do with a form submission (redisplay/preview/process).

    A valid submission would be presented to a form processor which would apply any domain logic then go and update persistent storage. You might be checking the total sales to a customer on each new purchase to see if they've qualified for a free waffle iron. There might be several classes involved - but if the domain is so simple that it's non-existent the submission would go straight to the db via whatever data access classes you use to update the relevant table/s.

    Designers can't manipulate html tied up in classes: for form display, I'd be looking at slotting dynamic data into html templates rather than a class which writes out the complete form html - but template systems are a whole new territory.

  11. #11
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Designers can't manipulate html tied up in classes: for form display, I'd be looking at slotting dynamic data into html templates rather than a class which writes out the complete form html - but template systems are a whole new territory.
    I think this is the main thing I disagree with, in the context of forms. It is of my opinion that there is only one proper way of marking up an HTML form. Therefore, I simply don't need access to the HTML. I can present the form however I want using CSS, as long as the structure is well defined (and I've made sure it is, XHTML is something I deal with day in day out).

    I don't know how to explain this any better at the moment, but I have a very strong feeling that "a Form class" should be separated into at least a validation and a display component.
    Perhaps I'm simply abstracting a form in a different way to everybody else. To me, a Form object contains Fieldset objects, and various other properties (id, method, action etc.). A method to check the form has been submitted (my form class is designed in such a way that whenever the form is submitted there will be a concrete way of checking its been submitted), a method to check the form is valid (there is more to this than just this method however), and a display method, which simply returns the form's HTML for processing by a separate class (this is all I need to do in terms of displaying a form, like I said, the HTML will not need to be modified as I believe an HTML form to be a rigid structure).

    The form's Fieldset objects then contain the Fields themselves. Only the Field objects (or other types of Field extended from this base Field class) know how they are to be validated. The Field classes contain the validation rules for that particular type of field. The Form object doesn't care how Field's are validated, it simply cares whether any of them have failed validation (and therefore, the form is invalid).

    For the purpose of Unit Testing, I'm of the view that you don't actually need to display a form.
    I thought the purpose of Unit Testing is to make sure that the code works, i.e. is doing what its supposed to be doing. Why should display or render methods be exempt from this testing?

    Display is presentation layer and validation is domain layer.
    Like I have tried to explain above, I believe that validation can be split in two...at the Field level, and at the Form level. A valid form is a form that contains no invalid fields. Field validation is not handled by the form, the form simply checks that all its Fields are valid. I cannot see any logical reason to encapsulate this very basic behaviour in a completely separate class.

    Likewise, with display, as I've said above it is my opinion, from an XHTML point of view, that a form has a rigid structure that more under semantics than pure presentation (which is delegated to CSS). So if the HTML does not vary for a particular field, why encapsulate this?

    A valid submission would be presented to a form processor which would apply any domain logic then go and update persistent storage.
    I agree and this is exactly how I'm trying to design it. Whilst not complete, this is my current FormController class:

    PHP Code:
    <?php
    /**
     *******************************************************
     * wildcat.forms.FormController
     * Controls the flow logic of any form
     *******************************************************
     * @author Luke Redpath
     * @version 1.0
     * @copyright Copyright &copy; Luke Redpath 2005
     *******************************************************
     */

    if (!defined('WILDCAT_PATH')) {
        
    define('WILDCAT_PATH''../');
    }
     
    /**
     * @package wildcat.forms

     */
    class FormController
    {
        
    //  MEMBERS
        
        /**
         * @var Form $_form the form to control
         */
        
    protected $_form;
        
        
    //  CONSTRUCTOR
        
        /**
         * Constructor
         * @param Form $formObject
         * @return void
         */
        
    function __construct(Form $form)
        {
            
    $this->_form $form;
        }
        
        
    //  MANIPULATORS

        /**
         * Run the controller
         * @return void
         */
        
    function run($outputBuffer)
        {        
            
    // check form submission
            
    if($this->_form->isSubmitted()) {

                
    // check form data is valid
                
    if($this->_form->isValid()) {
                    
    // process the form
                    /**
                     * NOT YET IMPLEMENTED
                     *
                       $formProcessor = $this->_form->getProcessor();
                       $formProcessor->execute();
                     */
                     
                    // render form successful view
                    
    $outputBuffer->addViewOutput('PAGEBODY'$this->_form->getSuccessView());
                } else {
                    
    // display submission failed view
                    
    $outputBuffer->addViewOutput('PAGEBODY'$this->_form->getFailedView());
                    
                    
    // redisplay the form
                    
    $outputBuffer->addContentString('PAGEBODY'$this->displayForm());   
                }
            } else {
            
                
    // form not submitted, display the form
                
    $outputBuffer->addContentString('PAGEBODY'$this->displayForm());
            
            }
        }
        
        private function 
    displayForm()
        {
            try {
                return 
    $this->_form->display();
            } catch (
    Exception $e) {
                return 
    $e->getMessage();
            }
        }
        
        
    //  ACCESSORS
    }
    ?>
    As you can see, the controller handles the logic of testing if a form is submitted/valid etc. If the form needs display, its output is returned so it can be handled by some kind of display system (i.e. at the simplest level it could just be echo'd, or it could be loaded into a page template as a whole block of code). If the form needs processing, the form is passed to a FormProcessor.

  12. #12
    simple tester McGruff's Avatar
    Join Date
    Sep 2003
    Location
    Glasgow
    Posts
    1,690
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Let's say I'm a designer working with the html. I want to set an image for a submit button. I want to apply styles to individual fields. I can't because I don't know php and so can't edit the classes. I also want to work entirely in Dreamweaver, using a Dreamweaver command to preview my work in a browser (whole pages at a time - no template fragments please). When I try, the form isn't present and the layout is all messed up. I don't want to have to upload every edit to a (specially created..) live test area to see if those 2px borders are too strong. I don't want to install a local server (even if I knew how) because it's too much hassle keeping the code in sync with the live site (the programmers said something about CVS: what is that?). Then there's the half gigabyte of data to download. The programmers said I can stub the db: I stroked my chin thoughtfully, nodded, and hoped nobody would notice my utter incomprehension.

    If only they'd concentrate on dynamic data and leave all the formatting to me...

  13. #13
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Sorry, you've still not got me convinced.

    First of all, I guess I have the privilege of being both the designer and the developer for a lot of my projects.

    Secondly, the class outputs enough HTML IDs/Classes to provide hooks for CSS styling. Its not a stretch of the imagination to generate the HTML for an example form which can be used for writing the CSS (the designer wouldn't even need to work with the live data). All they need to know is that the example form uses the same HTML markup as the live form will (although the fields might not be the same, this shouldn't matter), and that the live form will be dropped in the right place and styled with their CSS on the real site.

    They can simply paste in the example form HTML, then start styling it using CSS. As the CSS is completely separate from the content and markup, it doesn't matter whether the HTML is a static dump of an example form or the live dynamic PHP-driven form, the CSS will still be applied in the same way.

    Harsh, but I see no need to waste my time catering for those who can't write front-end code properly. The focus of this class isn't what will work for everybody, its what will work best for me with a pure web standards approach in mind (and others who share a similar mindset).

  14. #14
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    If only they'd concentrate on dynamic data and leave all the formatting to me...


    The way I look at it, I stated that there in my view, no need to display a form. As a web developer, my job is to ensure that the form validation and the process it's self works properly.

    I'm not (or should not at least) be concerned with the presentation. This (should) mean therefore that I wouldn't need to know about the form, ie The (x)HTML in this case, CSS has nothing to do with it - it's redundant, as I'm the developer.

    Someone else is the designer. But this is going off topic slightly I think as now this thread is moving towards separations of concerns.

    http://www.sitepoint.com/forums/showthread.php?t=111555

    Hope I've been of some help anyways

  15. #15
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    This (should) mean therefore that I wouldn't need to know about the form, ie The (x)HTML in this case, CSS has nothing to do with it - it's redundant, as I'm the developer.
    So you are essentially saying that as a developer you shouldn't care about the XHTML output?

    I don't see XHTML as being the form's presentation. AFAIC, the XHTML IS the form. Yes, there is validation, submission etc, but without the XHTML there is no form in the first place. I cannot see any good reason to separate them whatsoever. So in my mind, if I am writing a set of classes for creating forms, I should be developing something that ultimately returns that form itself ready to be plugged into the final output (how this is done is a separate issue).

  16. #16
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Dr Livingston
    I'm not (or should not at least) be concerned with the presentation. This (should) mean therefore that I wouldn't need to know about the form, ie The (x)HTML in this case, CSS has nothing to do with it - it's redundant, as I'm the developer.
    Again, I guess the benefit of being both the developer and the designer means I can develop classes that make my life as both a developer AND a designer easier.

  17. #17
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    I think your tests are mostly OK. I would suggest the odd small improvement to the tests and to the code.

    Code first. If you pass in the $_REQUEST hash to the form constructor then you won't have to jump through hoops to prevent test interference. This will help the clarity of the application as well...
    PHP Code:
    $form = &new Form('a_form''page.php'$_REQUEST); 
    As usual a cleaner test interface leads to more flexible code.
    PHP Code:
    $form = &new Form('a_form''page.php'$_POST); 
    ...is now possible.

    You don't need to set member variables in the test case so it is probably simpler to avoid it...
    PHP Code:
    function testSubmittingForm() {
        
    $form = new Form(array('submit' => 'Go!'));
        
    $fs1 $form->createFieldset('fs1''Some Fieldset');
        
    $fs1->addField('textfield''Text Field''text'true);
        
    $this->assertTrue($form->isSubmitted());        

    I think this is cleaner. It's certainly more explicit. Yes you will get some duplication, but tests are not the same as code. They also serve a purpose of documentation, and so unrolling them a little bit may make things clearer. It also means that each test can be specifically tuned. So I would wrie this...
    PHP Code:
    function testSendingSubmitEntryMarksFormAsSubmitted() {
        
    $form = new Form(array('submit' => 'Go!'));
        
    $this->assertTrue($form->isSubmitted());        

    As I assume an empty form is just as submittable as one with fields, this test is just as valid as the other. By being shorter, someone trying to figure out what went wrong when the test failed would have an easier time. Note the longer test name. I find it leads to more focussed tests.

    Here is something more outlandish.

    One other thing that makes me suspicious about the code is that form does it's stuff and is then queried with accessors. I would assume that the result of form validation would be some action to run, and the result of failure would be the old action. It makes more sense for the form to dispatch a message to the page controller rather than have this interchange of queries. Say that the current view is called PaymentFormView and the successful outcome is called WrapUpView, something like...
    PHP Code:
    function testFailedFormRedisplayesFormAgain() {
        
    $again = &new MockFormView($this);
        
    $again->expectOnce('reshow', array('name' => ''));

        
    $wrap_up = &new MockView($this);
        
    $wrap_up->expectNever('show');

        
    $form = new Form($again$wrap_up);
        
    $form->try(array('name' => '''submit' 'Go'));
        
    $again->tally();

    Might save fiddling around with controllers and the like. It also means that the form does more than just passively filter data, you save yourself accessor coe in other parts of the application and you are really testing a result. Asking the form if it is submitted smacks of an internal government enquiry. How can you trust theresult any more than the code. Just an idea.

    Anyway, yes you are on the right track withthe testing. Testing after writing the code is not nearly so much fun though .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  18. #18
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi Marcus,

    Thanks for your reply.

    I think this is cleaner. It's certainly more explicit. Yes you will get some duplication, but tests are not the same as code. They also serve a purpose of documentation, and so unrolling them a little bit may make things clearer. It also means that each test can be specifically tuned. So I would wrie this...

    PHP Code:
    function testSendingSubmitEntryMarksFormAsSubmitted() {
        
    $form = new Form(array('submit' => 'Go!'));
        
    $this->assertTrue($form->isSubmitted());        

    I see what you mean yes.

    Note the longer test name. I find it leads to more focussed tests.
    Right. It does make sense to do this - it just seems a little strange as in general when writing your actual non-test code its preferable to not have such verbose method names. I guess it takes a slight change in the approach to how you code when writing tests.

    Testing after writing the code is not nearly so much fun though
    You can say that again. I'm thinking about forgetting writing tests for my old code for the time being, and getting on with the rest of my classes using the test driven development style, then coming back to these later.

  19. #19
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    Quote Originally Posted by Luke Redpath
    I'm thinking about forgetting writing tests for my old code for the time being, and getting on with the rest of my classes using the test driven development style, then coming back to these later.
    I think that would be a good idea .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •