Verkorte code

Je zou een functie kunnen maken die je de veldnaam en de gewenste kleur meegeeft?

b.v.

function vak_inkleuren(veldnaam, kleurcode)
{
if(!$(veldnaam).val()){
     $(veldnaam).css("border-bottom-color", kleurcode);   
    }else{
     $(veldnaam).css("border-bottom-color", kleurcode);   
    }
}

Deze functie roep je dan x keer aan bijvoorbeeld.

Laat ik eerst beginnen met 2 algemene JavaScript best-practices:

• Probeer zo min mogelijk styling te doen, laat dat over aan CSS. Gebruik bijv. classes
• Probeer zo min mogelijk de hele DOM tree door te zoeken

Wanneer we de eerste best-practice toepassen maken we bijvoorbeeld 2 CSS classes aan:
[code lang=css].field--error { border-bottom-color:#ff0000; }
.field--default { border-bottom-color:#cccccc; }[/code]

Nu kunnen we deze toevoegen met jQuery:
[code lang=js]
if (!$('#dataeen').val()) {
$('#dataeen').removeClass('field-default').addClass('field-error');
} else {
$('#dataeen').removeClass('field-error').addClass('field-default');
}[/code]
Dit kunnen we versimpelen door jQuery's toggleClass() functie te gebruiken:
[code lang=js]// class wordt toegevoegd wanneer het 2e argument true is en wordt verwijderd wanneer dit false is
// dit betekend dat border-bottom-color:#cccccc; de default border-bottom kleur van de veldjes moet worden
$('#dataeen').toggleClass('field-error', !$('#dataeen').val());[/code]

Nu zie je dat we 2 keer $('#dataeen') hebben. Dit betekend dat JavaScript 2 maal alle elementen van de pagina (DOM tree) moet doorzoeken om een element te vinden. Je kan begrijpen dat dit met een grote pagina best wat tijd kan kosten. Het is dus beter om dit element op te slaan in een variabele en deze dan te gebruiken:
[code lang=js]
var dataeenElem = $('#dataeen');

dataeenElem.toggleClass('field-error', !dataeenElem.val());[/code]

Nu kunnen we dit nog beter maken, omdat de functie nu alsnog elke keer dat het wordt aangeroepen opzoek moet gaan naar de elementen. Dus kun je beter deze variabele globaal opslaan: (globaal is eigenlijk niet goed, maar dit lokaal maken gaat nu nog een beetje te ver)
[code lang=js]
var dataeenElem = $('#dataeen');
var datatweeElem = $('#datatwee');

$('#send').click(function (e) {
e.preventDefault();

dataeenElem.toggleClass('field--error', !dataeenElem.val());
datatweeElem.toggleClass('field--error', !datatweeElem.val());
});[/code]

Nu zie je dat je alsnog 2 regels hebt die nagenoeg hetzelfde zijn. Dit zou je ook weer kunnen abstraheren in een nieuwe functie, maar dat lijkt me een tikkeltje overkill.

Kijk trouwens eens naar het onsubmit event (.submit()). Dat is iets beter dan het click event op een submit button.

Het lijkt me gemakkelijker om alle elementen die je wilt controleren dezelfde class te geven.
Daarmee bereik je dat je niet elke id apart hoeft te vermelden:

je maakt een lus die door alle elementen met class="nietleeg" loopt en als zo'n element dan leeg is, geef je daar de opmaak class field-error aan. (oj juist niet aks gevuld)

Dank voor de reactie's.

Dit is wat ik nu heb jsfiddle
Is deze codering beter?

Ziet er zeker al gezond uit. Vraag me enkel af waarom je de class Default weghaalt? de class Error toevoegen of weghalen is al voldoende:

!$(this).val() ? $(this).addClass("error") : $(this).removeClass("error");

ik zou regel 10 zonder ! doen, en dan de 2 gevolgen omdraaien.

Scheelt je een (kleine) handeling voor javascript.

Je script stopt nu sowieso, ook als er wel alle velden ingevuld zijn. Is dat de bedoeling?

ik zou het schrijven als:

$("#data").on('submit',function() {
  $('.field').each(function(){
    $(this).val() ?  $(this).addClass("default").removeClass("error") : $(this).addClass("error").removeClass("default");
  });
  if($('.error').length > 0) { 
     return false;
   }
});   
  

Bedankt voor de reacties!

@ivo

Wat bedoel je precies met:

if($('.error').length > 0) { 
     return false;
   }

Betekend dat als de waarde meer dan 0 is dat hij de code niet weer opnieuw hoeft uit te voeren?
Of betekend "return false iets anders?

Aan het einde van de functie kijk ik of er uiteindelijk elementen aanwezig zijn met de class "error".
Als dat het geval is, zijn er dus elementen die leeg zijn en dus mogen we niet door met het submitten.

vergelijkbaar met preventDefault(). Maar jouw preventDefault gaat volgens mij altijd af.