-1

I have a function that works only its clunky and I know it could be written better, is their a way to combine the below?

$('#calculateAll').click(function(){        
    if( $('.a').val() === '' ) {
        $('.a').css('border-color','#BE1E2D');
        return false;
    } else if( $('.b').val() === '' ) {
        $('.b').css('border-color','#BE1E2D');
        return false;
    } else if( $('.c').val() === '' ) {
        $('.c').css('border-color','#BE1E2D');
        return false;
    } else if( $('.d').val() === '' ) {
        $('.d').css('border-color','#BE1E2D');
        return false;
    } else ...
Liam
  • 9,725
  • 39
  • 111
  • 209

4 Answers4

1

You can use the jQuery .each() function for this.

$('.a, .b, .c, .d').each(function() {
  if ($(this).val() == '') {
    $(this).css('border-color','#BE1E2D');
    return false;
  }
});
krisd
  • 58
  • 5
  • 1
    you should add a return false; statement in the if to have the same efect as Liam's code – Pinoniq Jul 18 '14 at 14:18
  • Just take note that this will not return false in the `$('#calculateAll').click(function() { .. });` function, but rather inside the `.each(function() { .. });` function. – David Sherret Jul 18 '14 at 14:40
0

Well it looks like you're validating something. Unlike the other answers, they forgot that there's more to it than just iterating the elements and giving them a red border if empty.

$('#calculateAll').on('click', function() {

  var err = false;
  // I would recommend using a class for these elements or a better selector
  $('.a, .b, .c, .d').each(function() {
    if ($(this).val() === '') {
      $(this).css('border-color','#BE1E2D');
      err = true;
      // If you don't want to add a border-color to the other elements, return false and break the loop
      return false;
    }
  });

  // Was something empty?
  if (err) {
    return false;
  }

  // All good, let's continue
  ...

});
Dennis Rasmussen
  • 520
  • 3
  • 12
  • This will not give he same result as his code. His code returns false the moment he has added a border ;) see krisd answer for the correct loop – Pinoniq Jul 18 '14 at 14:22
  • @Pinoniq True. I've added a return to the iteration and an explanation so he can choose either way. – Dennis Rasmussen Jul 18 '14 at 14:26
0

One possible shorter way of doing this is to do the following:

$('#calculateAll').click(function() {        
    var $invalidElements = $(".a, .b, .c, .d")
                           .css('border-color', '')
                           .filter(function() { return $(this).val() === "" }).first()
                           .css('border-color', '#BE1E2D');

    if ($invalidElements.length > 0) {
        return false;
    }
    else {
        // code executed when all elements are valid
    }
});

I recommend removing the .first() so that you can validate all the elements at the same time. Additionally, I added the line .css('border-color', '') to clear all the elements' border colours before validating.

David Sherret
  • 101,669
  • 28
  • 188
  • 178
-1

Demo: http://jsfiddle.net/U7t4p/1/

First find the empty inputs using .filter()

$('.container input[type="text"]').filter(function() { 
    return this.value === ''; 
})

This method iterates through all matched elements, if the callback returns true, the current element wilhttp://jsfiddle.net/U7t4p/1/l be added to the returned jQuery object.

At last we apply the CSS class to those elements:

.addClass('hl');
Chris Lam
  • 3,526
  • 13
  • 10