patternjavascriptMinor
JavaScript form validation
Viewed 0 times
javascriptformvalidation
Problem
Is this a "good" method of form validation?
width:550px;
}
.memb_col_0 {
width:550px;
float:left;
font-size:1.2em;
font-style:italic;
}
.memb_col_0_1 {
width:550px;
float:left;
padding-top: 10px;
}
.memb_col_1 {
width:140px;
float:left;
padding-top: 6px;
font-weight:bold;
}
.memb_col_2 {
width:410px;
float:left;
padding-top: 6px;
}
Persönliches:
Vorname
Christoph
Nachname
Eder
Spitzname
Geburtstag
Handynummer
Telegram-ID
123456
Wohnort
`
ori_edit_spitzname = $('#edit_spitzname').val();
ori_edit_gebtag = $('#edit_gebtag').val();
ori_edit_handy = $('#edit_handy').val();
ori_edit_herkunft = $('#edit_herkunft').val();
form_error = false;
form_changed = false;
// Spitzname
$('input[name=edit_spitzname]').change(function() {
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length
.memb_col {width:550px;
}
.memb_col_0 {
width:550px;
float:left;
font-size:1.2em;
font-style:italic;
}
.memb_col_0_1 {
width:550px;
float:left;
padding-top: 10px;
}
.memb_col_1 {
width:140px;
float:left;
padding-top: 6px;
font-weight:bold;
}
.memb_col_2 {
width:410px;
float:left;
padding-top: 6px;
}
Persönliches:
Vorname
Christoph
Nachname
Eder
Spitzname
Geburtstag
Handynummer
Telegram-ID
123456
Wohnort
`
Solution
Formatting
First of all with the excessive whitespace at the beginning of lines,
and the indenting that seems to follow no logic,
this is extremely difficult to read.
For example, this would be so much better:
Another awful formatting is here:
It would be so much easier to read like this:
Simplify
This condition can be simplified:
Because length greater than 0 and less than 2 is simply length that is exactly 1:
Avoid repeated dom lookups
Dom lookups like
and it's good to cache their result and reuse.
So instead of this:
This is recommended:
Use boolean expressions directly
Instead of:
You can use boolean expressions directly, and simply write:
Use style sheets
Instead of this:
It would be better to use a CSS class that applies these styles.
First of all with the excessive whitespace at the beginning of lines,
and the indenting that seems to follow no logic,
this is extremely difficult to read.
For example, this would be so much better:
$('input[name=edit_spitzname]').change(function() {
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length < 2) {
alert("Spitzname muss leer, oder minimal 2 Buchstaben lang sein!");
$('#edit_spitzname').css({'color': 'red', 'border-color' : 'red'});
$('#edit_spitzname').focus();
form_error = true;
} else {
$('#edit_spitzname').css({'color': 'green', 'border-color' : 'green'});
form_error = false;
}
is_changed_general();
});Another awful formatting is here:
if (ori_edit_spitzname!=$('#edit_spitzname').val()) { general_changed = true; }
else { $('#edit_spitzname').css({'color': 'black', 'border-color' : '#404040'}); }
if (ori_edit_gebtag!=$('#edit_gebtag').val()) { general_changed = true; }
else { $('#edit_gebtag').css({'color': 'black', 'border-color' : '#404040'}); }It would be so much easier to read like this:
if (ori_edit_spitzname != $('#edit_spitzname').val()) {
general_changed = true;
} else {
$('#edit_spitzname').css({'color': 'black', 'border-color' : '#404040'});
}
if (ori_edit_gebtag != $('#edit_gebtag').val()) {
general_changed = true;
} else {
$('#edit_gebtag').css({'color': 'black', 'border-color' : '#404040'});
}Simplify
This condition can be simplified:
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length < 2) {Because length greater than 0 and less than 2 is simply length that is exactly 1:
if ($('#edit_spitzname').val().length == 1) {Avoid repeated dom lookups
Dom lookups like
$('#edit_spitzname') are expensive,and it's good to cache their result and reuse.
So instead of this:
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length < 2) {
alert("Spitzname muss leer, oder minimal 2 Buchstaben lang sein!");
$('#edit_spitzname').css({'color': 'red', 'border-color' : 'red'});
$('#edit_spitzname').focus();This is recommended:
var edit_spitzname = $('#edit_spitzname');
if (edit_spitzname.val().length > 0 && edit_spitzname.val().length < 2) {
alert("Spitzname muss leer, oder minimal 2 Buchstaben lang sein!");
edit_spitzname.css({'color': 'red', 'border-color' : 'red'});
edit_spitzname.focus();Use boolean expressions directly
Instead of:
if (form_error==true) {You can use boolean expressions directly, and simply write:
if (form_error) {Use style sheets
Instead of this:
$('#edit_spitzname').css({'color': 'black', 'border-color' : '#404040'});
// ...
$('#edit_gebtag').css({'color': 'black', 'border-color' : '#404040'});It would be better to use a CSS class that applies these styles.
Code Snippets
$('input[name=edit_spitzname]').change(function() {
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length < 2) {
alert("Spitzname muss leer, oder minimal 2 Buchstaben lang sein!");
$('#edit_spitzname').css({'color': 'red', 'border-color' : 'red'});
$('#edit_spitzname').focus();
form_error = true;
} else {
$('#edit_spitzname').css({'color': 'green', 'border-color' : 'green'});
form_error = false;
}
is_changed_general();
});if (ori_edit_spitzname!=$('#edit_spitzname').val()) { general_changed = true; }
else { $('#edit_spitzname').css({'color': 'black', 'border-color' : '#404040'}); }
if (ori_edit_gebtag!=$('#edit_gebtag').val()) { general_changed = true; }
else { $('#edit_gebtag').css({'color': 'black', 'border-color' : '#404040'}); }if (ori_edit_spitzname != $('#edit_spitzname').val()) {
general_changed = true;
} else {
$('#edit_spitzname').css({'color': 'black', 'border-color' : '#404040'});
}
if (ori_edit_gebtag != $('#edit_gebtag').val()) {
general_changed = true;
} else {
$('#edit_gebtag').css({'color': 'black', 'border-color' : '#404040'});
}if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length < 2) {if ($('#edit_spitzname').val().length == 1) {Context
StackExchange Code Review Q#96646, answer score: 5
Revisions (0)
No revisions yet.