3
\$\begingroup\$

I have two forms in my HTML page. Both share a set of input elements (not a part of any of the forms, but rather residing separately). Besides that, the forms have their own input elements as well. Each of them also have their own notification div.

The process of POST request for both the forms is nearly identical, except those specifically owned input elements and notification divs.

Here is what I did to remove the code duplication:

function execute(config) {
    var initiator_token = getQueryVariable(window.location.search.substring(1), 'token');
    if (!initiator_token) {
        config.notifyEle
            .removeClass('alert-info')
            .addClass('alert-warning')
            .attr('data-i18n', config.invalidTokenMessage);
        return;
    }

    var params = {
        'initiator_token': initiator_token,
        'recipient_email': config.emailEle.val()
    };

    var lang = $('#mailLangSelector').val();
    if (lang) params['lang'] = lang;

    var message = $('#message').val();
    if (message) params['message'] = message;

    resetInputs();

    var loc = window.location;
    var url = loc.protocol + '//' + loc.host + $SCRIPT_ROOT + '/social' + config.url;

    $.ajax({
        url: url,
        type: 'POST',
        data: JSON.stringify(params),
        contentType: 'application/json',
        success: function(data, statusText, xhr) {
            config.notifyEle
                .removeClass('alert-warning')
                .addClass('alert-info')
                .attr('data-i18n', config.successMessage)
                .i18n();
        },
        error: function(xhr, statusText, errorThrown) {
            config.notifyEle
                .removeClass('alert-info')
                .addClass('alert-warning')
                .attr('data-i18n', config.failureMessage)
                .i18n();
        }
    });
}

So, the function accepts a config object, in which I pass form-specific values AND elements (I mean, jquery elements).

A config is like:

var inviteFormConfig = {
    emailEle: $('#inviteEmail'),
    notifyEle: $('#inviteInfo'),
    invalidTokenMessage: 'sharePage.descriptions.validTokenRequired',
    url: '/invitation',
    successMessage: 'sharePage.descriptions.invitationSent',
    failureMessage: 'sharePage.descriptions.invitationFailed'
};

And the usage for a given form:

$('#inviteForm').submit(function(evt) {
    evt.preventDefault();
    execute(inviteFormConfig);
});

Now, though this works well for my use, I want to know if there lies a better pattern for such a scenario. Also, should I be passing around jQuery elements like this in config?

\$\endgroup\$
0
1
\$\begingroup\$

This looks quite good, one nitpick I have is that you spend a significant amount of lines showing messages, that should be a more generic information setting function like

function notifyUser( messageClass, message ){
  config.notifyEle
        .removeClass( 'alert-info alert-warning' )
        .addClass( messageClass )
        .attr('data-i18n', message );   
}

And I must warn you that not putting curly braces is playing a bit with fire: if (lang) params['lang'] = lang; at least use a new line?

Edit: I would go for an MVC approach, create a Model object that knows what fields are on the form and knows how to validate them. Then a View object that knows how to style and update the form ( put red borders around blank input types, show messages etc. ) and a a Controller object which puts all the listeners in place and also takes care of working with the model to validate the data on the form before submitting web requests.

\$\endgroup\$
4
  • \$\begingroup\$ Nice touch there with the generic notify message. I have integrated that. Also, I agree with the concern of the lack of curly braces. I've added them. \$\endgroup\$ – Jayesh Bhoot Oct 31 '14 at 5:09
  • \$\begingroup\$ One question though. If it had been your code, would you have done this any other way? \$\endgroup\$ – Jayesh Bhoot Oct 31 '14 at 5:10
  • \$\begingroup\$ Updated with my approach \$\endgroup\$ – konijn Oct 31 '14 at 12:17
  • \$\begingroup\$ Alright, thanks! MVC approach seems interesting. I will give it a try, and may be put it here for review! \$\endgroup\$ – Jayesh Bhoot Nov 3 '14 at 4:04

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.