Validating a form without using a button and changing td class

Question

I need to validate the form inside a table and change the class of a td-element when the value is not set. I don't have a button inside a form and I can't use the jQuery validate script.

I have written this code but is there a way to do this in a simpler way?

I have written many pages with code like this.

<script>
function richiesti() {
 var dati=0;
 var cognome=document.clienti.cognome.value;
 if ( cognome != '' ){ dati++; } else { document.clienti.cognome.focus(); } 
 var nome=document.clienti.nome.value;
 if ( nome != '' ){ dati++; } else { document.clienti.nome.focus(); } 
 var codfisc=document.clienti.codfisc.value;
 if ( codfisc != '' ){ dati++; } else { document.clienti.codfisc.focus(); } 
 if ( dati == 3 ){ 
 // Se i tre valori richiesti sono inseriti controllo il codice fiscale
 // Definisco un pattern per il confronto
 var pattern = /^[a-zA-Z]{6}[0-9]{2}[a-zA-Z][0-9]{2}[a-zA-Z][0-9]{3}[a-zA-Z]$/;
 // creo una variabile per richiamare con facilità il nostro campo di input
 var CodiceFiscale = document.getElementById("codfisc");
 // utilizzo il metodo search per verificare che il valore inserito nel campo
 // di input rispetti la stringa di verifica (pattern)
 if (CodiceFiscale.value.search(pattern) == -1)
 {
 // In caso di errore stampo un avviso e pulisco il campo...
 alert("Il valore inserito non è un codice fiscale!");
 CodiceFiscale.value = "";
 CodiceFiscale.focus();
 }
 else { document.clienti.submit() ; }
 }
 else { alert('Cognome, Nome e Codice fiscale sono campi obbligatori.');
 if ( cognome == '' && nome == '' && codfisc == '' ){ 
 // Cambio la classe del td al valore mancante
 document.getElementById('tdcognome').className="tdorange c_white b";
 // Imposto la classe degli altri td nel caso sia stata cambiata
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdocra c_white b";
 document.getElementById('cognome').value="richiesto";
 document.clienti.cognome.focus(); } 
 if ( cognome == '' && nome != '' && codfisc != '' ){ 
 document.getElementById('tdcognome').className="tdorange c_white b";
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdocra c_white b";
 document.getElementById('cognome').value="richiesto";
 document.clienti.cognome.focus(); }
 if ( cognome == '' && nome == '' && codfisc != '' ){
 document.getElementById('tdcognome').className="tdorange c_white b";
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdocra c_white b";
 document.getElementById('cognome').value="richiesto"; 
 document.clienti.cognome.focus(); }
 if ( cognome == '' && nome != '' && codfisc == '' ){
 document.getElementById('tdcognome').className="tdorange c_white b";
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdocra c_white b";
 document.getElementById('cognome').value="richiesto";
 document.clienti.cognome.focus(); }
 if ( cognome != '' && nome == '' && codfisc != '' ){
 document.getElementById('tdcognome').className="tdocra c_white b";
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdorange c_white b";
 document.getElementById('nome').value="richiesto";
 document.clienti.nome.focus(); }
 if ( cognome != '' && nome == '' && codfisc == '' ){
 document.getElementById('tdcognome').className="tdocra c_white b";
 document.getElementById('tdcodfisc').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdorange c_white b";
 document.getElementById('nome').value="richiesto";
 document.clienti.nome.focus(); } 
 if ( cognome != '' && nome != '' && codfisc == '' ){
 document.getElementById('tdcognome').className="tdocra c_white b";
 document.getElementById('tdnome').className="tdocra c_white b";
 document.getElementById('tdcodfisc').className="tdorange c_white b";
 document.getElementById('codfisc').value="richiesto";
 document.clienti.codfisc.focus(); }
 }
 } 
</script>
<HTML>
<form name="clienti" id="clienti" method="POST" action="<?php echo $editFormAction; ?>">
<table class="half" >
 <tr>
 <td id="tdcognome" class="tdocra c_white b">Cognome :</td>
 <td><input name="cognome" id="cognome" type="text" class=" text-sx" value="" required></td>
 <td id="tdnome" class="tdocra c_white b">Nome :</td>
 <td><input name="nome" id="nome" type="text" class="text-sx" value="" required></td>
 <td id="tdcodfisc" class="tdocra c_white b">Codice Fiscale :</td>
 <td><input name="codfisc" id="codfisc" type="text" class="text-sx" value="" required></td>
 </tr>
 <tr>
 <td colspan="6"><a class="btn-orange" onClick="richiesti();" href="#">memorizza dati</a></td> 
 </tr>
 </table>
 </form>
</HTML>

After the suggestions i make many modify at original code.

REVIEW CODE :

<script>
function richiesti() { //open function richiesti
 var dati=0;
 var cognome=document.clienti.cognome;
 var nome=document.clienti.nome;
 var codfisc=document.clienti.codfisc;
 [cognome, nome, codfisc].forEach(function (field) { // open function field
 if (field.value != ''){ // open if value
 dati++;
 } // close if value
 else { // open else value
 field.focus();
 } // close else value
 }); // close function field
 if ( dati == 3 ){ // open if dati
 var pattern = /^[a-zA-Z]{6}[0-9]{2}[a-zA-Z][0-9]{2}[a-zA-Z][0-9]{3}[a-zA-Z]$/;
 var codiceFiscale = document.getElementById("codfisc");
 if (codiceFiscale.value.search(pattern) == -1) { // open if codiceFiscale
 alert("Il valore inserito non è un codice fiscale!");
 codiceFiscale.value = "";
 codiceFiscale.focus();
 } // close if codiceFiscale
 else { // open else codiceFiscale 
 document.clienti.submit() ;
 } // close else codiceFiscale
 } // close if dati 
 else { // open else dati
 alert('Cognome, Nome e Codice fiscale sono campi obbligatori.');
 if (cognome.value == '') { // open if cognome
 document.getElementById('tdcognome').className = "tdorange c_white b";
 document.getElementById('tdcodfisc').className = "tdocra c_white b"; 
 document.getElementById('tdnome' ).className = "tdocra c_white b";
 document.getElementById('cognome').value = "richiesto";
 cognome.focus();
 } // close if cognome
 else { // open else cognome
 document.getElementById('tdcognome').className="tdocra c_white b";
 if (nome.value == '') { // open if nome
 document.getElementById('tdcodfisc').className = "tdocra c_white b";
 document.getElementById('tdnome' ).className = "tdorange c_white b";
 document.getElementById('nome').value = "richiesto";
 nome.focus();
 } // close if nome
 else { // open else nome
 if (codfisc.value == '' ) { // open if codfisc
 document.getElementById('tdnome' ).className = "tdocra c_white b";
 document.getElementById('tdcodfisc').className = "tdorange c_white b";
 document.getElementById('codfisc').value = "richiesto";
 codfisc.focus();
 } // close if codfisc
 } // close else nome
 } // close else cognome
 } // close else dati 
} // close function richiesti 
</script>

Answer 1

Let's talk about this part:

var cognome=document.clienti.cognome.value;
if ( cognome != '' ){ dati++; } else { document.clienti.cognome.focus(); }

First of all, the indentation is awful. We can either do without braces

if (cognome != '') dati++;
else document.clienti.cognome.focus();

or preferably, put that conditional onto multiple lines.

if (cognome != ''){
 dati++;
} else {
 document.clienti.cognome.focus();
}

Now, you use both document.clienti.cognome.value and document.clienti.cognome.focus. It would be better to assign document.clienti.cognome to some variable, then:

var cognome = document.clienti.cognome;
if (cognome.value != ''){
 dati++;
} else {
 cognome.focus();
}

This allows us to remove repetition by looping over some values:

var cognome = document.clienti.cognome; 
var nome = document.clienti.nome;
var codfisc = document.clienti.codfisc;
[cognome, nome, codfisc].forEach(function (field) {
 if (field.value != ''){
 dati++;
 } else {
 field.focus();
 }
});

Later, you test for various combinations of fields being populated and assign classes depending on these combinations. The problem is that the classes do not always depend on all fields, so we could simplify that massive sequence of ifs:

if (cognome.value == '') {
 document.getElementById('tdcognome').className = "tdorange c_white b";
 document.getElementById('tdcodfisc').className = "tdocra c_white b"; 
 document.getElementById('tdnome' ).className = "tdocra c_white b";
 document.getElementById('cognome').value = "richiesto";
 cognome.focus();
}
else {
 document.getElementById('tdcognome').className="tdocra c_white b";
 if (nome.value == '') {
 document.getElementById('tdcodfisc').className = "tdocra c_white b";
 document.getElementById('tdnome' ).className = "tdorange c_white b";
 document.getElementById('nome').value = "richiesto";
 nome.focus();
 }
 else {
 if (codfisc.value == '' ) {
 document.getElementById('tdnome' ).className = "tdocra c_white b";
 document.getElementById('tdcodfisc').className = "tdorange c_white b";
 document.getElementById('codfisc').value = "richiesto";
 codfisc.focus();
 }
 }
}

Oh look, the body is always the same if cognome == ''! Because we now cleaned up the conditions, we can also see that some cases have been missed: what happens if cognome != '' && nome != '' && codfisc != ''? In cases like this it would be helpful to write a comment explaining that no classes will be changed.

---

There are a few more comments to be made on your style:

• Avoid non-English comments and variable names. I had difficulty understanding your code because I do not speak Italian. English is spoken by virtually all programmers, so it's a better choice for maintainable code.

• It seems your indentation has been messed up by copying the code here. However, there are one very bad thing:

  if (cond) {
   statement;
   last_statement }
  

  Place the closing curly brace on a line of it's own, this makes it easier to see.

• You have one variable that's uppercase: CodiceFiscale. Stick to a consistent naming scheme, preferably camelCase.

• Your code could become much shorter by using jQuery. It's a de-facto standard, and helps abstracting over many compatibility issues.

• Using == instead of === is usually frowned upon, as == tries to coerce the values to some mathching type. Depending on what you're trying to do, this is actually preferable, but most of the time it's a code smell. The === operator first asserts that both values are of the same type.

Answer 2

Here are some minor refactoring tips. You should put this function into a class/object, if you are repeating your code. And then import the file onto each html page that requires this validation.

You shouldn't check if the variable is empty this way

if ( cognome != '')
{
 //code goes here
}

The better way is the following:

// this way checks to see if the variable is null and/or empty
if (congnome)
{
//code goes here
}

You could also move the following into its own function document.getElementById('tdnome').className="tdocra c_white b";

For example:

function changeClassName(elementId, newClassValue)
{
 if (elementId)
 {
 document.getElementById(elementId).class = newValue;
 }
}

Then you can just call it.

changeClassName('tdnome', '"tdocra c_white b');

• geomo: The validation happen here : <a class="btn-orange" onClick="richiesti();" href="#">memorizza dati</a>