2

Terminology

Element/Form Element -> Fields such as text list, combo_boxes, push buttons, etc.

Problem

I have a program that is basically a fat client that interfaces into a system that keeps track of traffic lights.

In this program exists a form. The form contains several subforms. As it is right now, some of the subforms have their own classes, and some of the subforms are just controlled completely from the class of the main form.

The main form class has a validate method which is called when the form is submitted and calls validation methods within the subforms, as well as performing validation on some of the form elements that don't have their own classes.

I'm not sold on this architecture, as I feel that it should be consistent (every subform should have its own class, or no subform should have it's own class.

My problem lies in the implementation of handling warnings. Right now, when an element is validated against, the validation results in an error or a warning. If it is an error, the user must correct the box to meet the forms validation requirements. If it is a warning, the user must be notified of that warning, and then submit the form a second time.

If a warning occurs, I have created a flag that will be set. Before the information within the form elements can be accepted, the form first checks if any errors were thrown. In the event there are no errors, it checks for warnings. If the warning flag is set, it displays the warning. The next time this method is called, the warning flag is toggled within the validation method.

I don't really like this implementation, as it basically means you have this rogue flag that should be cleared based on the requirements. I've thought about implementing it as a state machine, however you run into the same problem where the state of the form would be set in various spots within the class based on the requirements (One of the fields changed, reset to the starting state, etc).

The other problem that I ran into is that the warning flag is owned by the main forms class. I can create a pointer to the parent form from the subform, and then set the flag from there.

This all feels very clunky, and I feel like it could create a lot of confusion. To me there has to be a better way to architect this.

EDIT: Adding some code for context

bool MainForm::callback_pushbutton_submit_clicked( const FormEvent&)
{
    if ( validate_all_forms() )
    {
        //display error
    }
    else if ( get_warning_displayed_flag() )
    {
        //display warning
    }
    else
    {
        //submit form
    }

}

bool MainForm::validate_all_forms()
{   
    bool error = false;
    bool display_warning = false;
    error |= subform1.validate();
    error |= subform2.validate();
    //...

    //perform some validation on subforms that are controlled within the main form
    if ( !(my_date_field.validate() )
    {
        display_warning = true;
    }
    //....

    if( error )
    {
        //format error message
        //clear warning flag
    }
    else if ( display_warning )
     if ( !get_display_warning_flag() )
     {
        //get warning message
     }
     invert_warning_display_flag()
}
return error;
2

I'm going to quote your question and add some code to it (it's easier to speak about it afterwards):

In this program exists a form. The form contains several subforms. As it is right now, some of the subforms have their own classes, and some of the subforms are just controlled completely from the class of the main form.

struct subform { virtual bool validate() const = 0; };

struct warning { element& f; string message; }
struct error { /* subform or element reference list, and a message here */ };

The main form class has a validate method which is called when the form is submitted and calls validation methods within the subforms, as well as performing validation on some of the form elements that don't have their own classes.

class form {
    vector<unique_ptr<subform>> subforms;

    bool is_valid() const
    {
        auto sf_valid = [](const unique_ptr<subform>& sf){
            return sf->validate();
        });
        if(all_of(begin(subforms), end(subforms), sf_valid))
        {
            // custom *this validation code
            return true;
        }
        return false;
    }
    void update_data()
    {
        // update all data here, based on the content of the
        // forms and fields in *this
    }
public:
    void submit()
    {
        if(is_valid())
        {
             update_data();
             // submit form here (whatever that means)
        }            
    }

};

This architecture is written for the positive case only. It is up to you if you want to treat warning and error as exceptions, or not.

If you treat them as exceptions, throw instances of them in subform::validate and in form::is_valid, and catch and handle all that, in form::submit()`.

If you treat them as non-exceptional cases (i.e. part of normal application flow and not actual errors), then instead of returning bool from subform::validate and form::is_valid, return a user-defined type that converts to bool, and has accessors for any error(s) and warning(s) issued.

In submit, on the else branch or in a catch block, you should reset the fields and subforms referenced in the received error or warning instances.

Edit (explanation):

You mentioned getting rid of the flag for error/success. This implementation splits the validation from the commit phase (updating the data and submitting), and the validation is const (the functions do not alter state, and propagate information by return value/throwning).

You can then (if needed), refactor the custom validation code in form into new subforms.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.