best way to refactor my form (procedural to oop?)

Question

(Note: this is related to this question, but I think it could have been written more clearly, so I'm trying again -- my update only helped to a limited extent.)

I've inherited some code that creates a complex form with numerous sections, and lots of possible views, depending on a number of parameters. I've been working with it for a while, and finally have a chance to think about doing some re-factoring. It's currently written procedurally, with a bunch of functions that look like this:

get_section_A ($type='foo', $mode='bar', $read_only=false, $values=array()) {
    if ($this->type == 'foo') { 
        if ($this->mode == 'bar') { }
        else { }
    } else { }
}

Passing around those parameters is nasty, so I've started writing a class like this:

class MyForm {
    public $type;          // or maybe they'd be private or 
    public $mode;          // I'd use getters and setters 
    public $read_only;     // let's not get distracted by that :)
    public $values;
    // etc.

    function __constructor ($type='foo', $mode='bar', $read_only=false, $values_array=array()) {
        $this->type = $type;
        // etc.
    }

    function get_sections () {
        $result = $this->get_section_A();
        $result .= $this->get_section_B();
        $result .= $this->get_section_C();        
    }      

    function get_section_A() { 
        if ($this->type == 'foo') { }
        else { }
    }
    function get_section_B() {}
    function get_section_C() {}
    // etc.
}

The problem is that the procedural functions are split into a few files (for groups of sections), and if I combine them all into a single class file, I'm looking at 2500 lines, which feels unwieldy. I've thought of a few solutions:

1. keep living with the nasty parameters and do something else with my time :)
2. live with having a 2500 line file
3. create a separate class for each group of sections that somehow "knows" the values of those parameters

If I do #3, I've thought of two basic approaches:

1. pass the MyForm object in as a single parameter
2. create a FormSectionGroup class with static properties that get set in MyForm, then in the group files, each class would extend FormSectionGroup and automatically have access to the current values for those parameters.

1) is probably easier to set-up, and once I'm inside get_section_A() whether I say $this->type or $myForm->type isn't all that different, but it's not exactly OOP. (In fact, I could do that without really changing to an OOP approach.)

Are there other approaches? Thoughts about which is better?

Answer 1

I thought of this when I posted in your previous question - this problem reeks of decorator pattern.

It's going to be no small task, though. But I think you'll have an amazing sense of satisfaction/accomplishment once you get it done.

Answer 2

Having done a lot of Cocoa programming recently, I tend to view things in MVC-pattern (Model-View-Controller) terms. Hence, I'd look at the form as a controller of its various sections.

Each section object should be responsible for keeping track of its status, values and whether or not it should be displayed. Or to be precise, the section_model would take care of the values (default values, validation, etc), the section_view would take care of displaying (or not) parts of the section and the section_controller would send keep track of the status of the section and report results to the form object.

The form object should instantiate the section controllers, tell them to display or hide or whatever, and get status reports. The form object, really act a controller, can then decide when the form if completely filled out. You may have a form_model object to save the collected data, or maybe you'd rather have the section_model objects take part of that. It will take a while to get a feeling for how the different objects interact but I know from experience that if you are disciplined in designing your objects (the key is: what is an object's responsibility and what is not), you will gain a better overview and your code will be easier to upgrade. Once you find that improvements start to arise naturally, you are on the right track.

Answer 3

I would love nothing more than to write a lengthy explanation of how to do this, but I'm feeling a bit lazy. I do however have enough energy to point you instead to Zend_Form from the zend framework. There may be some dependencies to make it work properly (Zend_View, Elements, Decorators), but once you have them, it handles that type of situations quite gracefully.

Answer 4

If you have the time to work on doing #3, you will probably be the happiest over the long run. Personally, I don't have the time in my job to refactor to that degree very often, and so I'd probably be forced to pick #1. #2 sounds like the worst of both worlds to me. Lots of work to get code you don't like.