tdc
tdc

Reputation: 5464

how to optimize repeated if/else blocks?

This is a fragment of validation code on an onSubmit event:

/**
 * ensure user name was provided:
 */
if (user_name.val() === '') {

    // display error on the name element 
    user_name.parent().addClass('error');
    user_name.parent().find('.error_msg').html('Name is required.');
    error = true;

} else {

    // reset error state
    user_name.parent().removeClass('error');
    user_name.parent().find('.error_msg').empty();

}

/**
 * Ensure user email was provided:
 */
if (user_email.val() === '') {

    // display error on the email element
    user_email.parent().addClass('error');
    user_email.parent().find('.error_msg').html('Email is required.');

} else {

    // reset error state 
    user_email.parent().removeClass('error');
    user_email.parent().find('.error_msg').empty();

}

/**
 * Ensure Message Subject was provided 
 */
if (message_subject.val() === '') {

    // display error on the subject element
    message_subject.parent().addClass('error');
    message_subject.parent().find('.error_msg').html('Message subject is required.');

} else {

    message_subject.parent().removeClass('error');
    message_subject.parent().find('.error_msg').empty();

}

/**
 * Ensure Message Body was provided
 */
if (message_body.val() === '') {

    // display error on the message body element
    message_body.parent().addClass('error');
    message_body.parent().find('.error_msg').html('A message is required.');

} else {

    message_body.parent().removeClass('error');
    message_body.parent().find('.error_msg').empty();

}

As you can see, lots of repeated if/else blocks with simple logic.

What would be a good way to optimize this code to reduce overall size while still accomplishing the same behaviors?

Upvotes: 1

Views: 165

Answers (4)

lshettyl
lshettyl

Reputation: 8171

Here is another way of doing what you asked for - empty check. Write a function that would take an object of options and function would do the rest. Bear in mind that this is only a demo and you may have to change things around to suit your needs.

function validate(opts) {
    var isError = false;
    $.each(opts.fields, function(field, msg) {
        var $field = opts.form.find("input[name='" + field + "']");
        var $parent = $field.parent();
        if ( $.trim($field.val()) === "") {
            $parent.addClass(opts.errorToggleClass).
            find(opts.errorMsgClass).html(msg);
            isError = true;
        } else {
            $parent.removeClass(opts.errorToggleClass).
            find(opts.errorMsgClass).empty();
        }
    });
    return isError;
}

$(".go").on("click", function(evt) {

    var isError = validate({
        form: $(".myForm"),
        fields: {
            "user_name": 'Name is required.',
            "user_email": 'Email is required.',
            "message_subject": 'Message subject is required.',
            "message_body": 'A message is required.'    
        },
        errorToggleClass: 'error',
        errorMsgClass: '.error_msg'
    });
    alert ("Error: " + isError);
});
.error {
    color: red;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<form class="myForm">
    <div>
        <p class="error_msg"></p>
        <input name="user_name" type="text" />
    </div>
    <div>
        <p class="error_msg"></p>
        <input name="user_email" type="text" />
    </div>
    <div>
        <p class="error_msg"></p>
        <input name="message_subject" type="text" />
    </div>
    <div>
        <p class="error_msg"></p>
        <input name="message_body" type="text" />
    </div>
    <input class="go" type="button" value="Submit" />
</form>

Upvotes: 0

Rick Hitchcock
Rick Hitchcock

Reputation: 35670

I would let CSS do most of the work.

Put the error messages within the HTML like this:

<div>
  <input id="user_name" type="text">
  <span class="error_msg">Name is required.</span>
</div>

Make them hidden by default, but visible if their parent has an error class:

.error_msg {
  display: none;
}

div.error .error_msg {
  display: inline;
}

Your code then reduces to this:

$('input, textarea').change(function() {
  $(this).parent('div').toggleClass('error', !$(this).val());
}).change();

Fiddle

Upvotes: 1

Mat&#237;as Fidemraizer
Mat&#237;as Fidemraizer

Reputation: 64931

Honestly, you could simplify it developing a validation engine, but wait! There're a plenty of validation libraries. For example, ParsleyJS (there're many ones...).

Using a validation library will supress the lines instead of simplifying them, and many of these validation libraries implement declarative validation (i.e. they use HTML attributes like required, min-length, max-length...) so there will near to zero JavaScript code to perform form validation.

Of course, you could also simplify your code using a function like some other answer suggests to... but again, honestly, if you're developing a Web site or app which has forms and you need to validate them (I would say you must validate forms!!), I wouldn't waste my time performing form validation manually. Keep it simple.

Upvotes: 0

kasimoglou
kasimoglou

Reputation: 384

I would make the following function and I would call it for every field:

function validation_check(obj, msg) {
    if (obj.val() === '') {
       obj.parent().addClass('error');
       obj.parent().find('.error_msg').html(msg);
    } else {
       message_body.parent().removeClass('error');
       message_body.parent().find('.error_msg').empty();
    }
}

Upvotes: 0

Related Questions