Return to Answer

added 1304 characters in body

Source Link

edited Jul 12, 2015 at 8:28

janos

113.1k
15
154
396

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

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.

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

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

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.

Source Link

answered Jul 12, 2015 at 8:18

janos

113.1k
15
154
396

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:

$('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();     
});

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