0
\$\begingroup\$

I am new to jquery and implemented following function in a js file in case of error-

function before_submit_check_if_has_blank_field() {
    $("[type=submit]").click(function(e) {
        var of_key_field = $('.key_field')
        var of_val_field = $('.val_field')
        var should_run = 1

        if (of_key_field.length === 1) {
            $('.key_field:not(:first)').filter(function() {
                if (this.value === "") {
                    alert('Please Fill In Or Remove The Blank Fields First')
                    e.preventDefault();
                    should_run = 0
                }
            });
        } else {
            $('.key_field').filter(function() {
                if (this.value === "") {
                    alert('Please Fill In Or Remove The Blank Fields First')
                    e.preventDefault();
                    should_run = 0
                }
            });
        }

        if (should_run != 0) {
            if (of_val_field.length === 1) {
                $('.val_field:not(:first)').filter(function() {
                    if (this.value === "") {
                        alert('Please Fill In Or Remove The Blank Fields First')
                        e.preventDefault();
                        should_run = 0
                    }
                });
            } else {
                $('.val_field').filter(function() {
                    if (this.value === "") {
                        alert('Please Fill In Or Remove The Blank Fields First')
                        e.preventDefault();
                        should_run = 0
                    }
                });
            }
        }

        update_config_field_from_key_value_field()
    });
}

The above code snippet is a part of my jquery where I am displaying a layout to the user with some fields and once those fields are empty then I am showing alert message while saving the form.

In above function I am repeating following lines of alert message which must be replaced by standard one-

if (this.value === "") {
    alert('Please Fill In Or Remove The Blank Fields First')
    e.preventDefault();
    should_run = 0
}

Can anyone please review my code and refactor it?

\$\endgroup\$
  • \$\begingroup\$ Hey, welcome to Code Review! Your current title says what you want out of a review, but since we all want to improve our code here one way or another, you should edit it to describe what problem your code solves. In addition, some plain English description of that in the question body would also make this a bit clearer. \$\endgroup\$ – Graipher Jul 6 '18 at 9:10
  • \$\begingroup\$ @Graipher I have added summary of my task. \$\endgroup\$ – AnalyticsPy Jul 6 '18 at 9:17
  • \$\begingroup\$ @AnalyticsPy: The title of the question should state what the code does. Can you change this? \$\endgroup\$ – Greg Burghardt Jul 6 '18 at 11:08
1
\$\begingroup\$

Your code can be refactor in multiple ways. To remove code repetition, create a new function for repeated code and use it as a callback of each filter.

function someFunc(e) {
  if (this.value === "") {
    alert('Please Fill In Or Remove The Blank Fields First')
    e.preventDefault();
    should_run = 0
  }
}

Use above function as

$('.key_field:not(:first)').filter(someFunc);

This will automatically remove redundant code as well as reduce number lines.

Suggestions:

Instead of playing around DOM and get value for each field separately, try to use serializeArray method of JQuery. See example

Please make habit to use ; to terminate line.

Hope this is helpful.

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

What I would do is to create an object error = { errorCode: false} and every time you run throw check you assign to errorCode true if the conditions are not met.

Then at the end you just check if error.errorCode is true and display the error message.

\$\endgroup\$

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.