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>

amon · Accepted Answer · 2014-02-01 17:30:18Z

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.

James · Accepted Answer · 2014-02-01 13:37:42Z

3

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');

edited Feb 1, 2014 at 13:37

answered Feb 1, 2014 at 12:30

James

3481 silver badge12 bronze badges

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

geomo
– geomo

2014-02-01 12:56:19 +00:00
Commented Feb 1, 2014 at 12:56

Add a comment |

Stack Exchange Network

Validating a form without using a button and changing td class

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Validating a form without using a button and changing td class

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions