1
\$\begingroup\$

I'm trying to make my code easier to understand by making it more correct and maintainable.

Here's the code I'm trying to simplify:

       $(document).ready(function(){
  $('.opretbruger').on('click', function(){
    if (opretbruger() === true){

    }
  });
});

var opretbruger = function(){
  if($('#brugernavn').val() === ''){
     alert('remember Email');
      return false;
  }

  if($('#pass1').val() === ''){
    alert('remember password');
    return false;
  }

  if($('#pass2').val() === ''){
    alert('remember password');
    return false;
  }

  if($('#fornavn').val() === ''){
    alert('remember your name');
    return false;
  }

  if($('#efternavn').val() === ''){
    alert('remember your last name');
    return false;
  }

  return true;
};

Here's my attempt at simplifying it:

       $(function () {
     var $login = $("#login"),
         $pwd = $("#pass"),
         $usr = $("#brugernavn");

     $login.on("submit", function (event) {
         var msg = "Husk ",
             usr = ($usr.val().trim() !== ""),
             pwd = ($pwd.val().trim() !== "");

         msg += !usr ? " brugernavn" : "";
         msg += !usr && !pwd ? " og " : "";
         msg += !pwd ? " kodeord" : "";

         (pwd && usr) || alert(msg);

         return (pwd && usr);
     });
 });
\$\endgroup\$
2
  • \$\begingroup\$ What does overskulig mean? \$\endgroup\$ Commented Dec 20, 2013 at 22:06
  • \$\begingroup\$ @Nobody its foreseeable. :) \$\endgroup\$ Commented Dec 20, 2013 at 22:07

2 Answers 2

2
\$\begingroup\$

Let me make sure I understand the problem. You have a number of fields on the page where a value is required; when the user submits the page, and any of the fields are blank, you'd like the page to display a Javascript alert instead of submitting the form. Yes?

I'd suggest you use an unobtrusive technique as follows:

  1. If a field is required, it is marked with a "required" css class. This can be a dummy class if you like.
  2. Each field that is required is associated with a data attribute which specifies its human-readable name in the appropriate language
  3. The submit button inspects all fields that are marked with the style and displays the names.

So to mark up a required field, use this pattern:

<input ID="login" class="required" data-name="login"/>

Then include this bit of code in your common script. There's nothing page- or field- specific in it, so you can include it in every page if you want (e.g. in "common.js" or whatever):

$(document).ready(function(){
    var validate = function(e){
        var emptyFields = [];
        $(".required").each(function(){
            if ($(this).val().trim() == "") emptyFields.push($(this).data("name")); 
        }
        if (emptyFields.length != 0) {
            e.preventDefault();
            var message = "You forgot these fields: " + emptyFields.join(","); 
            alert(message);
        }
    }

    $(":submit").click(validate);
})

This ought to cover any additional fields or really any additional pages that require this sort of validation.

\$\endgroup\$
3
  • \$\begingroup\$ its not work for me, It does not come up with some errors or similar \$\endgroup\$ Commented Dec 20, 2013 at 23:05
  • \$\begingroup\$ I fixed a minor syntax error (missing a right paren), please try again. If you still have trouble, please let me know what kind of error. \$\endgroup\$
    – John Wu
    Commented Dec 20, 2013 at 23:42
  • \$\begingroup\$ it works is still not, and have not error.. \$\endgroup\$ Commented Dec 20, 2013 at 23:52
1
\$\begingroup\$

Solution 1

  1. I suggest you place your inputs inside a <form>, that way, you won't be listening for a button click or ID each input. Instead, you'd be listening for a form submit. You can easily do that this way:

  2. On the HTML side of things, the inputs in the forms need name attributes like they should. This will come handy later in the code

  3. Next is to get this very handy snippet. This turns your form into an object, with keys as the names of the form inputs, and values as their values. I have to note that this snippet has issues when names have dashes, so avoid them.

  4. Now, for the messages, you should make a map for the names to messages. Then in your validation code, loop through the messages and look at the value of the same key in formValues. If it the value is blank, then alert the message.

The code should look like this all in all. All you need to do is add names and messages for the things you want to check to messages.

var messages = {
  brugeravn : 'You forgot brugeravn',
  pass1 : 'You forgot pass1',
  ...
}

$('#TheFormID').on('submit',function(event){
  event.preventDefault();
  var formValues = $(this).serializeObject();

  $.each(messages,function(key,message){
    if(formValues.hasOwnProperty(key) && formValues[key] === ''){
      alert(message);
    }
  });
});

Solution 2

This would be similar to John Wu's answer, but a bit refined. I usually do this when developing with backend guys who don't like to write JS. So I give them config capability through data-* attributes.

  1. Same as solution 1, put everything in a form for ease of handling.

  2. On the HTML, add in a few parameters. name isn't required this time.

     <input type="text" data-required="true" data-message="You forgot brugeravn" />
    

    As you can see, we set the input to "required" and set in a message, both via data-* attributes.

The only JS code you'll need for this solution is the following. What it does is listen for the form, gather all inputs marked "required" and check their values. If it's blank, alert their supplied message.

    $('#TheFormID').on('submit',function(event){
      event.preventDefault();

      $(this).find('[data-required=true]').each(function(){
        if(this.value === '') alert(this.getAttribute('data-message'));
      });
    });
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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